5

I have added data into ArrayList and now want to update that list be deleting some element from it.

I have element something like 1,2,3,4 in ArrayList of type CartEntry.

Code :

ArrayList<CartEntry> items = new ArrayList<CartEntry>();

public void remove(int pId)
{
    System.out.println(items.size());

    for(CartEntry ce : items)
    {
        if(ce.getpId() == pId)
        {
            items.remove(ce);
            //System.out.println(items.get(1));             
        }
    }   
    items.add(new CartEntry(pId));
}

CartEntry Code :

public long getpId() {
    return pId;
}

Constructor :

public CartEntry(long pId) {
    super();
    this.pId = pId;     
}

when I am trying this code it gives me an error:

java.util.ConcurrentModificationException
    at java.util.ArrayList$Itr.checkForComodification(Unknown Source)
    at java.util.ArrayList$Itr.next(Unknown Source)

Here pId is the argument that specify that item should be deleted from items. Suppose I want to delete item that have 2 data then what will I have to do ?

Improve this question
6
  • 1
    What goes wrong with the code you have? Commented Nov 21, 2013 at 9:16
  • 1
    Don't remove without iterator. This is dangerous. Commented Nov 21, 2013 at 9:17
  • @MarounMaroun - Sir I have posted exception which I got and How to implement iterator ? Commented Nov 21, 2013 at 9:18
  • why are you adding the CartEntry again "items.add(new CartEntry(pId));" in the remove function Commented Nov 21, 2013 at 9:19
  • Just iterate the list in the reverse order for(int i=items.count; i <=0; i--) Commented Nov 21, 2013 at 9:22

6 Answers 6

Reset to default
14

You are facing ConcurrentModificationException because you are doing two operations on the same list at a time. i.e looping and removing same time.

Inorder to avoid this situation use Iterator,which guarantees you to remove the element from list safely .

A simple example looks like

Iterator<CartEntry> it = list.iterator();
    while (it.hasNext()) {
        if (it.next().getpId() == pId) {
            it.remove();
            break;
        }
    }
Improve this answer
Sign up to request clarification or add additional context in comments.

4 Comments

Superb Answer.. Everyone's Answer Are Same and Your One line extra Made my code easy and workable and that is a simple break. Thanx.
@user2659972ღ: the break is not necessary and only realy useful if you know that no two CartEntry objects will have the same id.
@JoachimSauer -Yes sir but If I don't use break and delete item then it works but after that I got the error same as above but problem item is removed. SO that's why break make it done for me.
@user2659972ღ: if you get the exception without the break then you're still using items.remove() as opposed to it.remove()!
3

There are at least two problems with your code:

  1. you call remove on the collection you iterate over, that will cause a ConcurrentModificationException if you continue iterating after the remove.

    There are two ways to fix this:

    • stop iterating after you found the object you want to remove (just add a break or a return) or
    • switch from the enhanced for-loop to using an Iterator and its remove method.

  2. you add an element in your remove method, that's probably not what you want.

So I'd use this code (this assumes that there is only ever one CartEntry with a given id in the list):

public void remove(int pId)
{
    for(CartEntry ce : items)
    {
        if(ce.getpId() == pId)
        {
            items.remove(ce);
            return;
        }
    }   
}

If the assumption with the unique id is not correct, then you'll need to use the Iterator approach:

public void remove(int pId)
{
    Iterator<CartEntry> it = items.iterator();
    while(it.hasNext())
    {
        CartEntry ce = it.next();
        if(ce.getpId() == pId)
        {
            it.remove();
        }
    }   
}
Improve this answer

Comments

3

you have created an Arraylist of type carEntry. So you need to create an Iterator of type CarEntry

Iterator<CarEntry> it =  items.iterator();    
while(it.hasNext())
{
   if(it.next().getPId == PId)
   it.remove();
}
Improve this answer

Comments

1

Implement .equals in CartEntry and then use ArrayList.remove(CartEntry) or loop through your array list, find the item with some condition, mark the index, and call ArrayList.remove(index) -- AFTER the loop

Improve this answer

Comments

1

Try,

public void remove(int pId){

Iterator<CartEntry> it = items.iterator();

 while(it.hasNext()) {
    CartEntry entry = it.next();
    if (entry.getpId() == pId) {
        it.remove();
    }
  }
}
Improve this answer

Comments

1

The enhanced-for(or for each) loop for iterating over an Expression which is a subtype of Iterable<E> or raw Iterable, basically equivalent to the following form:

  for (I #i = Expression.iterator(); #i.hasNext(); ) {   
      VariableIdentifiers_opt TargetType Identifier = (TargetType) #i.next();
         Statement
   }

This is clearly stated in jls 14.14.2. The enhanced for statement section.

For your context the Expression is an ArrayList. the iterators returned by ArrayList's iterator method is fail-fast: if the list is structurally modified at any time after the iterator is created, in any way except through the iterator's own remove or add methods, the iterator will throw a ConcurrentModificationException.

use an Iterator instead and use it's own remove() method:

  Iterator<E>iterator = list.iterator();
  while(iterator.hasNext())
    if(iterator.next().equals(E))
      iterator.remove();
Improve this answer

Comments

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.