Bigflow
Bigflow

Reputation: 3666

Delete data from ArrayList with a For-loop

I got a weird problem. I thought this would cost me few minutes, but I am struggling for few hours now... Here is what I got:

for (int i = 0; i < size; i++){
    if (data.get(i).getCaption().contains("_Hardi")){
        data.remove(i);
    }
}

The data is the ArrayList. In the ArrayList I got some strings (total 14 or so), and 9 of them, got the name _Hardi in it.

And with the code above I want to remove them. If I replace data.remove(i); with a System.out.println then it prints out something 9 times, what is good, because _Hardi is in the ArrayList 9 times.

But when I use data.remove(i); then it doesn't remove all 9, but only a few. I did some tests and I also saw this:

When I rename the Strings to: Hardi1 Hardi2 Hardi3 Hardi4 Hardi5 Hardi6

Then it removes only the on-even numbers (1, 3, 5 and so on). He is skipping 1 all the time, but can't figure out why.

How to fix this? Or maybe another way to remove them?

Upvotes: 23

Views: 53752

Answers (14)

Drazen Bjelovuk
Drazen Bjelovuk

Reputation: 5472

In addition to the existing answers, you can use a regular while loop with a conditional increment:

int i = 0;
while (i < data.size()) {
    if (data.get(i).getCaption().contains("_Hardi"))
        data.remove(i);
    else i++;
}

Note that data.size() must be called every time in the loop condition, otherwise you'll end up with an IndexOutOfBoundsException, since every item removed alters your list's original size.

Upvotes: 4

Burak Cakir
Burak Cakir

Reputation: 928

It is late but it might work for someone.

Iterator<YourObject> itr = yourList.iterator();

// remove the objects from list
while (itr.hasNext())
{
    YourObject object = itr.next();
    if (Your Statement) // id == 0
    {
        itr.remove();
    }
}

Upvotes: 3

Taslim Oseni
Taslim Oseni

Reputation: 6263

This is a common problem while using Arraylists and it happens due to the fact that the length (size) of an Arraylist can change. While deleting, the size changes too; so after the first iteration, your code goes haywire. Best advice is either to use Iterator or to loop from the back, I'll recommend the backword loop though because I think it's less complex and it still works fine with numerous elements:

//Let's decrement!
for(int i = size-1; i >= 0; i--){
    if (data.get(i).getCaption().contains("_Hardi")){
        data.remove(i);
    }
 }

Still your old code, only looped differently!

I hope this helps...

Merry coding!!!

Upvotes: 3

JMarsch
JMarsch

Reputation: 21753

Every time you remove an item, you are changing the index of the one in front of it (so when you delete list[1], list[2] becomes list[1], hence the skip.

Here's a really easy way around it: (count down instead of up)


for(int i = list.size() - 1; i>=0; i--)
{
  if(condition...)
   list.remove(i);
}

Upvotes: 13

Fergus
Fergus

Reputation: 471

There is an easier way to solve this problem without creating a new iterator object. Here is the concept. Suppose your arrayList contains a list of names:

names = [James, Marshall, Susie, Audrey, Matt, Carl];

To remove everything from Susie forward, simply get the index of Susie and assign it to a new variable:

int location = names.indexOf(Susie);//index equals 2

Now that you have the index, tell java to count the number of times you want to remove values from the arrayList:

for (int i = 0; i < 3; i++) { //remove Susie through Carl
    names.remove(names.get(location));//remove the value at index 2
}

Every time the loop value runs, the arrayList is reduced in length. Since you have set an index value and are counting the number of times to remove values, you're all set. Here is an example of output after each pass through:

                           [2]
names = [James, Marshall, Susie, Audrey, Matt, Carl];//first pass to get index and i = 0
                           [2]
names = [James, Marshall, Audrey, Matt, Carl];//after first pass arrayList decreased and Audrey is now at index 2 and i = 1
                           [2]
names = [James, Marshall, Matt, Carl];//Matt is now at index 2 and i = 2
                           [2]
names = [James, Marshall, Carl];//Carl is now at index 3 and i = 3

names = [James, Marshall,]; //for loop ends

Here is a snippet of what your final method may look like:

public void remove_user(String name) {
   int location = names.indexOf(name); //assign the int value of name to location
   if (names.remove(name)==true) {
      for (int i = 0; i < 7; i++) {
         names.remove(names.get(location));
      }//end if
      print(name + " is no longer in the Group.");
}//end method

Upvotes: 2

Dariusz R.
Dariusz R.

Reputation: 96

I don't understand why this solution is the best for most of the people.

for (Iterator<Object> it = data.iterator(); it.hasNext();) {
    if (it.next().getCaption().contains("_Hardi")) {
        it.remove();
    }
}

Third argument is empty, because have been moved to next line. Moreover it.next() not only increment loop's variable but also is using to get data. For me use for loop is misleading. Why you don't using while?

Iterator<Object> it = data.iterator();
while (it.hasNext()) {
    Object obj = it.next();
    if (obj.getCaption().contains("_Hardi")) {
            it.remove();
    }
}

Upvotes: 4

hariees
hariees

Reputation: 21

import java.util.ArrayList;

public class IteratorSample {

    public static void main(String[] args) {
        // TODO Auto-generated method stub

        ArrayList<Integer> al = new ArrayList<Integer>();
        al.add(1);
        al.add(2);      
        al.add(3);
        al.add(4);

        System.out.println("before removal!!");
        displayList(al);

        for(int i = al.size()-1; i >= 0; i--){
            if(al.get(i)==4){
                al.remove(i);
            }
        }

        System.out.println("after removal!!");
        displayList(al);


    }

    private static void displayList(ArrayList<Integer> al) {
        for(int a:al){
            System.out.println(a);
        }
    }

}

output:

before removal!! 1 2 3 4

after removal!! 1 2 3

Upvotes: 2

Sumit.Joshi
Sumit.Joshi

Reputation: 51

Its because when you remove an element from a list, the list's elements move up. So if you remove first element ie at index 0 the element at index 1 will be shifted to index 0 but your loop counter will keep increasing in every iteration. so instead you of getting the updated 0th index element you get 1st index element. So just decrease the counter by one everytime you remove an element from your list.

You can use the below code to make it work fine :

for (int i = 0; i < data.size(); i++){
    if (data.get(i).getCaption().contains("_Hardi")){
        data.remove(i);
        i--;
    }
}

Upvotes: 5

Ramesh PVK
Ramesh PVK

Reputation: 15446

The Problem here is you are iterating from 0 to size and inside the loop you are deleting items. Deleting the items will reduce the size of the list which will fail when you try to access the indexes which are greater than the effective size(the size after the deleted items).

There are two approaches to do this.

Delete using iterator if you do not want to deal with index.

for (Iterator<Object> it = data.iterator(); it.hasNext();) {
if (it.next().getCaption().contains("_Hardi")) {
    it.remove();
}
}

Else, delete from the end.

for (int i = size-1; i >= 0; i--){
    if (data.get(i).getCaption().contains("_Hardi")){
            data.remove(i);
    }
 }

Upvotes: 60

Subhrajyoti Majumder
Subhrajyoti Majumder

Reputation: 41200

for (Iterator<Object> it = data.iterator(); it.hasNext();) {
    if ( it.getCaption().contains("_Hardi")) {
        it.remove(); // performance is low O(n)
    }
}

If your remove operation is required much on list. Its better you use LinkedList which gives better performance Big O(1) (roughly).

Where in ArrayList performance is O(n) (roughly) . So impact is very high on remove operation.

Upvotes: 3

yshavit
yshavit

Reputation: 43391

It makes perfect sense if you think it through. Say you have a list [A, B, C]. The first pass through the loop, i == 0. You see element A and then remove it, so the list is now [B, C], with element 0 being B. Now you increment i at the end of the loop, so you're looking at list[1] which is C.

One solution is to decrement i whenever you remove an item, so that it "canceles out" the subsequent increment. A better solution, as matt b points out above, is to use an Iterator<T> which has a built-in remove() function.

Speaking generally, it's a good idea, when facing a problem like this, to bring out a piece of paper and pretend you're the computer -- go through each step of the loop, writing down all of the variables as you go. That would have made the "skipping" clear.

Upvotes: 4

matt b
matt b

Reputation: 139931

You shouldn't remove items from a List while you iterate over it. Instead, use Iterator.remove() like:

for (Iterator<Object> it = list.iterator(); it.hasNext();) {
    if ( condition is true ) {
        it.remove();
    }
}

Upvotes: 20

Michael Laffargue
Michael Laffargue

Reputation: 10304

Because your index isn't good anymore once you delete a value

Moreover you won't be able to go to size since if you remove one element, the size as changed.

You may use an iterator to achieve that.

Upvotes: 3

WojtekT
WojtekT

Reputation: 4775

This happens because by deleting the elements you modify the index of an ArrayList.

Upvotes: 2

Related Questions