CODEWITHSUNDEEP
javaarraylist
babygau
babygau

Reputation: 1581

Remove item from ArrayList issue

I'm writing a class that have a method of removing an object from other class. But it just worked inproperly, the output is not correct. Please help me through, and is there any better solution for this, I think my solution is quite cumbersome. Here is my code:

public List<Task> getTaskDue(){        
    List<Task> temp = this.taskCollection;
    for (int unit = 0; unit < this.unitCollection.size(); unit++){            
        for (int j = 0; j < this.unitCollection.get(unit).getAssessmentCollection().size(); j++){
            for (int i = 0; i < temp.size(); i++){
                if (temp.get(i).getDueDate().compareTo(this.unitCollection.get(unit).getAssessmentCollection().get(j).getDueDate()) > 0)
                    temp.remove(i);
            }
        }  
    }
    return temp;
}

Updated: I have Diary class that has list of Task class and Assessment class that hold due date attribute. I want to create a method that return a new list which have a list of over due task by comparing the task from diary class with the due date attribute from assessment class. The program compile successfully but the result is not correct if I want to test the list return no task item since no task is over due.

Upvotes: 0

Views: 507

Answers (3)

Jeremy
Jeremy

Reputation: 22415

It seems like "removing" elements from the list isn't your ultimate problem.

You said you want your method to return a new list that contains elements from taskCollection based on some criteria. At the same time, I don't think you want to destroy or change taskCollection in any way.

So instead of creating temp as a reference to taskCollection, have it be a new ArrayList<Task>() instead. Then add tasks to temp (the new list) that you want to ultimately return from your method.

I am going to leave my adivce at that, because your code sample, in isolation, has a lot of unknowns that prohibit me from making any educated guesses on what you really need it to do.

Also, there are too many for loops! (I'm mostly kidding, but seriously...)

With more information from the comments below, I've modified your code to implement what I am suggesting. In order to add the items to temp (instead of remove them) I had to change your if statement from > 0 to <= 0. Also, instead of iterating over taskCollection in the inner-most loop, you should get tha tasks from the current assessment and iterate over those.

public List<Task> getTaskDue(){     
    List<Task> temp = new ArrayList<Task>();

    for(int u = 0; u < unitCollection.size(); u++){
        Unit unit = unitCollection.get(u);

        for (int a = 0; a < unit.getAssessmentCollection().size(); a++){
            AssessmentItem assessment = unit.getAssessmentCollection().get(a);

            for (int t = 0; t < assessment.getTasks().size(); t++){
                Task task = assessment.getTasks().get(t);

                if (task.getDueDate().compareTo(assessment.getDueDate()) <= 0){ 
                    temp.add(task);
                }   
            }   
        }   
    }   
    return temp;
}

Upvotes: 3

Ravinder Reddy
Ravinder Reddy

Reputation: 23992

You are removing an object from an index but index in iteration is not altered. Due to the reason you would be skipping an element in the list. Perhaps that is the reason why your results are not correct.

Change: 

if  ( temp.get( i ).getDueDate().compareTo( this.unitCollection.get( unit )
      .getAssessmentCollection().get( j ).getDueDate() ) > 0 )
    temp.remove(i);

to: 

if  ( temp.get( i ).getDueDate().compareTo( this.unitCollection.get( unit )
      .getAssessmentCollection().get( j ).getDueDate() ) > 0 )
{  
    temp.remove(i);  
    i--;  
}

PS: Better always practice using flower braces irrespective of number of statements under the condition or loop.

Upvotes: 0

Neil Coffey
Neil Coffey

Reputation: 21795

If you need to alter a list as you're iterating through it, use a ListIterator. Call listIterator() on your list to create one, then see relevant methods on ListIterator.

Upvotes: 1

Related Questions