Sam kh
Sam kh

Reputation: 127

IndexOutOfBoundsException: I don't see in my while loop

Ive been stuck on this IndexOutOfBoundsException that I can't figure out and I don't see why it's pointing to my while loop. Any suggestions on what to do? The method btw is a round robin scheduling simulator, it seems to be working perfectly with one of my testfiles but not the other.

public void roundRobin2(ArrayList<Jobs> c, int startSize)
{   
    //int n = 0;
    double counter = 0;
    double compTime = 0;
    while(!c.isEmpty())
    {
        int i = 0;
        System.out.println(c);
        for(i = 0; i < c.size(); i++)
        {
            if((c.get(i).jobTime) >= 2)
            {
                c.get(i).jobTime -= 2;
                counter += 2;

                if((c.get(i).jobTime) == 0)
                {
                    compTime += counter;
                }
            }
            else
            {
                (c.get(i).jobTime) -= 1;        
                counter += 1;

                if((c.get(i).jobTime) == 0)
                {
                    compTime += counter;
                }
            }
            //System.out.print("-" + c.get(i).jobName + "-" + counter);
            //n++;
            //if(n%10 == 0)
            //{
                //System.out.println("\n");
            //}
        }
        for(i = 0; i < c.size(); i++)
        {
            while(!c.isEmpty() && (c.get(i).jobTime) == 0)
            {
                c.remove(i);
            }       
        }
    }
    System.out.println("\n\nAverage completion times: "+ compTime + "/" + startSize +" = " + ((compTime)/startSize));
}

The exception stacktrace:

Exception in thread "main" java.lang.IndexOutOfBoundsException: Index: 7, Size: 7
    at java.util.ArrayList.rangeCheck(Unknown Source)
    at java.util.ArrayList.get(Unknown Source)
    at cs431.edu.cpp.Main.roundRobin2(Main.java:143)

Upvotes: 1

Views: 409

Answers (2)

Dici
Dici

Reputation: 25960

You are removing items in a collection you are iterating through at the same time, this is very bad and you should never do that. You can do so using an Iterator which has a remove method (however this is rarely efficient), or make a temporary copy of your collection. By the way round-robin can be implemented much more easily.

This solution uses a LinkedList because removal at the head and insertion at the end are both O(1), this is efficient and safe. I also compressed some of your logic as a bonus :

public void roundRobin2(List<Jobs> jobs, int startSize) {
    double counter = 0;
    double compTime = 0;

    Deque<Jobs> uncompletedJobs = new LinkedList<>(jobs);
    while (!uncompletedJobs.isEmpty()) {
        Job job = uncompletedJobs.pop();
        int taskTime = job.jobTime >= 2 ? 2 : 1;
        job.jobTime -= taskTime;
        counter += taskTime;
        if (job.jobTime == 0) {
            compTime += counter;
        } else {
             uncompletedJobs.addLast(job);
        }
    }
    jobs.clear();
    System.out.println("\n\nAverage completion times: "+ compTime + "/" + startSize +" = " + ((compTime)/startSize));
}

Upvotes: 1

Andreas
Andreas

Reputation: 159106

Your code:

while(!c.isEmpty() && (c.get(i).jobTime) == 0)
{
    c.remove(i);
}

will fail if i is removing the last element, but not causing the list to go empty. Use this instead:

while (i < c.size() && c.get(i).jobTime == 0)
{
    c.remove(i);
}

Update

Your code would clean up a lot by using the for (obj : list) loop, and you should remove elements using an Iterator. Here's the cleaned up code:

public static void roundRobin2(ArrayList<Jobs> c, int startSize)
{
    double counter = 0;
    double compTime = 0;
    while (! c.isEmpty())
    {
        System.out.println(c);
        for (Jobs jobs : c)
        {
            if (jobs.jobTime >= 2)
            {
                jobs.jobTime -= 2;
                counter += 2;
            }
            else
            {
                jobs.jobTime -= 1;
                counter += 1;
            }
            if (jobs.jobTime == 0)
            {
                compTime += counter;
            }
        }
        for (Iterator<Jobs> jobsIter = c.iterator(); jobsIter.hasNext(); )
        {
            Jobs jobs = jobsIter.next();
            if (jobs.jobTime == 0)
            {
                jobsIter.remove();
            }
        }
    }
    System.out.println("\n\nAverage completion times: " + compTime + "/" + startSize + " = " + (compTime / startSize));
}

Upvotes: 2

Related Questions