Error with java.util.ConcurrentModification while adding one bookmark

Question

I got problem when i add the last 7st item position, other 6 items work perfectly with auto removes whenhasEnded. I cant add the 7st item, and when i got other items like 1-6 and adding the 7st item my application got error:

java.util.ConcurrentModificationException

one the code:

 public List<LiveTvProgram> getLiveTvBookmarks() {
    String name = context.getString(R.string.preferences_bookmarks_live_tv);
    Set<String> bookmarks = preferenceHelper.getStringSet(name, name);
    ArrayList<LiveTvProgram> bookmarkList = new ArrayList<>();
    for (String title : bookmarks) {
        for (LiveTvProgram program : ChannelsManager.getInstance().getSelectedPrograms()) {
            if (program.getTitle().equals(title)) {
                if (program.hasEnded()) {
                    bookmarks.remove(title);
                } else {
                    bookmarkList.add(program);
                }
                break;
            }
        }
    }
    return bookmarkList;
}

With first for loop

for (String title : bookmarks) {
        for (LiveTvProgram program : ChannelsManager.getInstance().getSelectedPrograms()) {
            if (program.getTitle().equals(title)) {
                if (program.hasEnded()) {
                    bookmarks.remove(title);
                } else {
                    bookmarkList.add(program);
                }
                break;
            }
        }
    }

score 2 · Accepted Answer · edited May 23 '17 at 12:00

You get this exception because you are trying to modify the list bookmarks while you are iterating over it by calling bookmarks.remove(title). Please have a look at the answer of this question to see how to avoid that: How to avoid java.util.ConcurrentModificationException when iterating through an removing elements from an ArrayList

On a side note: Are you sure that you really need to remove items from the bookmarks list at all? You generate that list in your method by calling

Set<String> bookmarks = preferenceHelper.getStringSet(name, name);

and as you do not do anything with this list after you iterate over it may be you could just drop removing.

UPDATE: A quick general suggestion to rewriting your loops:

for (LiveTvProgram program : ChannelsManager.getInstance().getSelectedPrograms()) {
    //Check if this program is in your bookmarks. As bookmarks is 
    //a Set, contains should be quite efficient
    if (bookmarks.contains(program.getTitle()) {                
        bookmarkList.add(program);
        //further improvement: you could check here if all bookmarks 
        //have been matched and exit the loop if yes
    }
}

If you really need to clean your bookmarks list, you need to retain all entries which are not included in bookmarkList after this loop.

loicmathieu · Answer 2 · 2016-08-22T08:13:19.003

You cannot iterate on a list and modify it, it will always lead to ConcurrentModificationException.

So your code is incorrect, you cannot iterate over bookmarks and remove one of it's elements in the loop.

for (String title : bookmarks) {
        for (LiveTvProgram program : ChannelsManager.getInstance().getSelectedPrograms()) {
            if (program.getTitle().equals(title)) {
                if (program.hasEnded()) {
                    bookmarks.remove(title);//FORBIDDEN !!!
                } else {
                    bookmarkList.add(program);
                }
                break;
            }
        }
    }

UPDATED with a working example :

    List<String> bookmarksToRemoved = new ArrayList<>();
    for (String title : bookmarks) {
            for (LiveTvProgram program : ChannelsManager.getInstance().getSelectedPrograms()) {
                if (program.getTitle().equals(title)) {
                    if (program.hasEnded()) {
                        bookmarksToRemoved.add(title);//keep them in a temporary list
                    } else {
                        bookmarkList.add(program);
                    }
                    break;
                }
            }
        }
    bookmarks.removeAll(bookmarksToRemoved);//remove all bookmarks to be removed

Regards,

Loïc

Hello, Do you have maybe any example how to do this correctly? — Genehme, Aug 22 '16 at 07:55
You should copy the bookmarks to be removed in a temporay List and removing them after the loop. I will edit my answer to reflect that — loicmathieu, Aug 22 '16 at 08:10
Yea this working, i thinking about making 2nd list with removing items and updating my first list with changes — Genehme, Aug 22 '16 at 08:20

score 0 · Answer 3 · answered Aug 22 '16 at 08:16

0

You can not remove the item of the list which you are iterating with in for each loop. So please ignore this for better solutions.

answered Aug 22 '16 at 08:16

Virendra Pal Singh

313
3
13

Error with java.util.ConcurrentModification while adding one bookmark

3 Answers3