Removing ArrayList object issue

Question

I got an issue with deleting an object from ArrayList when working on the assignment If I use the "normal" for loop, it works as following

public void returnBook(String isbn){        
    for (int i = 0; i < booksBorrowed.size(); i++){            
        if (booksBorrowed.get(i).getISBN() == isbn){
            booksBorrowed.get(i).returnBook();
            booksBorrowed.remove(i);                
        }
    }
}

However, when I'm trying to simplify the code with enhanced for-loop, that doesn't work and showing java.util.ConcurrentModificationException error:

public void returnBook(String isbn){        
        for (Book book: booksBorrowed){            
            if (book.getISBN() == isbn){
                book.returnBook();
                booksBorrowed.remove(book);                
            }
        }
}

Hope you guys could lighten me up..

Answer 1

Your alternatives to avoid a ConcurrentModificationException are:

List<Book> books = new ArrayList<Book>();
books.add(new Book(new ISBN("0-201-63361-2")));
books.add(new Book(new ISBN("0-201-63361-3")));
books.add(new Book(new ISBN("0-201-63361-4")));

Collect all the records that you want to delete on enhanced for loop, and after you finish iterating, you remove all found records.

ISBN isbn = new ISBN("0-201-63361-2");
List<Book> found = new ArrayList<Book>();
for(Book book : books){
    if(book.getIsbn().equals(isbn)){
        found.add(book);
    }
}
books.removeAll(found);

Or you may use a ListIterator which has support for a remove method during the iteration itself.

ListIterator<Book> iter = books.listIterator();
while(iter.hasNext()){
    if(iter.next().getIsbn().equals(isbn)){
        iter.remove();
    }
}

Or you may use a third-party library like LambdaJ and it makes all the work for you behind the scenes>

List<Book> filtered = select(books, 
                having(on(Book.class).getIsbn(), 
                        is(new ISBN("0-201-63361-2"))));

Answer 2

You really shouldn't be doing either as they will cause problems in the end. Instead use the ArrayList's iterator to help you iterate through the list and then remove only with the iterator. This will help prevent pernicious concurrent modification errors.

Answer 3

All good answers. But I would sugest you to rethink it. I mean, do you really need a ArrayList or a HashMap would be better? If your list of objects have a unic key (ISBN), and you use it to get each object, why not use a collection appropriated for your problem?

You woud do only this

public void returnBook(String isbn){        
     Book book = (Book) booksBorrowed.remove(isbn);            
     book.returnBook();    
}