14

I have following code-

import java.util.ArrayList;

public class ArrayListExp{
    public static void main (String[] args){

        ArrayList<String> name = new ArrayList<String>();

        name.add("Chris");
        name.add("Lois");
        name.add("Meg");
        name.add("Meg");
        name.add("Brain");
        name.add("Peter");
        name.add("Stewie");

        System.out.println(name);

        for ( int i = 0;  i < name.size(); i++){
            String oldName = name.get(i);
            if(oldName.equals("Meg"))
            {
                name.remove(i);
            }
        }

        System.out.println(name);
    }
}

But here it gives me output -

[Chris, Lois, Meg, Meg, Brain, Peter, Stewie]
[Chris, Lois, Meg, Brain, Peter, Stewie]

I am not getting the point, why this is not removing Meg but I have tried with only one Meg in that case it is working. And I when I am adding few more Meg in last the one Meg is not removed from the ArrayList. Why?

Unmitigated
  • 76,500
  • 11
  • 62
  • 80
CodeCrypt
  • 711
  • 2
  • 8
  • 20
  • You must use an iterator to remove in the middle of a loop. This is covered often. http://stackoverflow.com/questions/13847695/java-delete-arraylist-iterator – zero298 Oct 19 '13 at 02:23
  • 2
    I've seen this snippet at least 2 times before this evening. If you look trough the java submissions of the past few hours you'll find a lot of helpful remarks that will help your exact case. – Jeroen Vannevel Oct 19 '13 at 02:24

7 Answers7

25

When you remove the first "Meg", the index i=2. Then it's incremented, but since one of the "Meg" is already removed, now name.get(3) is "Brain". So you didn't actually check the second "Meg".

To fix the problem. you can decrement the index when you remove an element:

public class ArrayListExp{
    public static void main (String[] args){

        ArrayList<String> name = new ArrayList<String>();

        name.add("Chris");
        name.add("Lois");
        name.add("Meg");
        name.add("Meg");
        name.add("Brain");
        name.add("Peter");
        name.add("Stewie");

        System.out.println(name);

        for ( int i = 0;  i < name.size(); i++){
            String oldName = name.get(i);
            if(oldName.equals("Meg"))
            {
                name.remove(i);
                i--;
            }
        }

        System.out.println(name);
    }
}
tianz
  • 2,155
  • 2
  • 19
  • 28
7

You are iterating over the first Meg, and when that Meg gets removed, the array values shift over by one.

[Chris, Lois, Meg, Meg, Brain, Peter, Stewie]
   0     1     2    3     4      5       6

First Meg gets removed, and the loop increments i because it finished executing everything inside the for loop, so i will now be 3 and the array has been modified:

[Chris, Lois, Meg, Brain, Peter, Stewie]
   0     1     2     3      4      5      

Try iterating backwards.

for ( int i = name.size() - 1;  i >= 0; i--){
    String oldName = name.get(i);
    if(oldName.equals("Meg"))
    {
        name.remove(i);
    }
}
dtgee
  • 1,272
  • 2
  • 15
  • 30
4

You can use name.removeAll(Arrays.asList("Meg")); to remove all "Meg"

Your complete code would be

for ( int i = 0;  i < name.size(); i++){
    String oldName = name.get(i);
    if(oldName.equals("Meg"))
    {
       name.removeAll(Arrays.asList("Meg"));
    }
}
Prabhakaran Ramaswamy
  • 25,706
  • 10
  • 57
  • 64
1

You're removing from the ArrayList while iterating over it from 0 to N, so when you remove the first Meg at index N, the next Meg moves down to index N, then you increment i to N+1. So the 2nd Meg doesn't get removed. Try iterating in the opposite order (N to 0):

for ( int i = name.size() - 1;  i >= 0; i--) {
Mark Vayngrib
  • 974
  • 5
  • 14
1

Its because when i=2 and if condition is true then meg is deleted and all the indices are shifted up. hence the next i will point to Brain, not meg.

try this. (decrease i by one when if condition holds true)

for ( int i = 0;  i < name.size(); i++){
            String oldName = name.get(i);
            if(oldName.equals("Meg"))
            {
                name.remove(i);
                i--;
            }
        }
Anurag Rana
  • 1,429
  • 2
  • 24
  • 48
0

While removing elements you should not use for loop. It always make problems when implementing some logic. Use reverse for loop for your problem and always try to use for each.

Kanagaraj M
  • 956
  • 8
  • 18
0

Java 8+

The Collection interface now provides a removeIf method that modifies it in place, to which you provide a predicate that returns true if the element should be removed.

You can thus use a lambda like so:

name.removeIf(name -> name.equals("Meg"));

A method reference can be used as well to be more concise. The following code will also work if there are null elements.

name.removeIf("Meg"::equals);

If you do not want to modify the old list, you can use Stream and filter to get a List of all the items that should be retained by negating the condition.

final List<String> filtered = name.stream()
      .filter(name -> !"Meg".equals(name))
      .collect(Collectors.toList());

If you specifically need an ArrayList, use Collectors.toCollection with a constructor reference instead.

final ArrayList<String> filtered = name.stream()
       .filter(name -> !"Meg".equals(name))
       .collect(Collectors.toCollection(ArrayList::new));

Pre Java 8

The issue is that the ArrayList is being modified while being iterated over, which changes its size and shifts later elements forward; this is a problem if there are consecutive elements that need to be removed. You need to decrease the index by one each time you remove an element since that index will now refer to the next element.

for (int i = 0; i < name.size(); i++) {
    String oldName = name.get(i);
    if (oldName.equals("Meg")) {
        name.remove(i);
        i--;//Important!
    }
}

Looping backwards will also fix this problem, as elements will never be shifted to a position that have yet to be checked.
for (int i = name.size() - 1; i >= 0; i--) {
    String oldName = name.get(i);
    if (oldName.equals("Meg")) {
        name.remove(i);
    }
}

Generally, using an Iterator is the most appropriate for this sort of operation, as it supports removing elements while iterating through calling Iterator#remove. This also modifies the List in place. For more complex operations, consider using a ListIterator.

final Iterator<String> it = name.iterator();
while(it.hasNext()){
    final String name = it.next();
    if(name.equals("Meg")){
         it.remove();
    }
}
Unmitigated
  • 76,500
  • 11
  • 62
  • 80