Reputation: 127
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
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
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