How to avoid ConcurrentModificationException on recursive iterator based removal from LinkedList?

Question

I've been trying to efficiently create trees by assigning a child to its parent and then deleting it via it.remove():

for (OgOrganizationTreeDTO parent : parents) {
    setChildren(parent, organizationTree);
}

Here is the implementation of the setChildren function

private void setChildren(OgOrganizationTreeDTO root, List<OgOrganizationTreeDTO> allOrganizations) {
    if (allOrganizations.isEmpty()) {
        return ;
    }
    Iterator<OgOrganizationTreeDTO> it = allOrganizations.iterator();
    while (it.hasNext()) {
        OgOrganizationTreeDTO potentialChild = it.next();
        if (potentialChild.getIdParentId() != null && potentialChild.getIdParentId().equals(root.getId())) {
            root.addChild(potentialChild);
            it.remove();
            setChildren(potentialChild, allOrganizations);
        }
    }
}

I am using a LinkedList and I get a ConcurrentModificationException. I've solved the problem by passing a copy of allOrganizations to the recursive setChildren function like so new LinkedList<>(allOrganizations), but the copy part takes O(n) time and I don't want that.

I've also tried using LinkedBlockingQueue, but I've found out that deletion takes O(n) time.

I want to take advantage of LinkedList O(1) remove, so everytime I add a child, and recur on it the list becomes smaller.

I've also successfully implemented a solution with HashSet by marking individual nodes as seen, and setting the base case to hashSet.size() == allOrganizations.size(), but I still keep recurring over the same size List, so this doesn't help me other.

Is there any way of accomplishing my goal of using LinkedList O(1) remove, or is there an even more efficient alternate approach to this ?

Or if you worry about performance you can do it without recursion - use a big hashmap to store all processed entites and you can do it with 2 iterations on the list — Veselin Davidov, Feb 14 '19 at 10:38
Possible duplicate of [loop on list with remove](https://stackoverflow.com/questions/1921104/loop-on-list-with-remove) — Sid, Feb 14 '19 at 10:39

Veselin Davidov · Accepted Answer · 2019-02-14T10:57:23.537

Well I don't know how to do it with recursion because you actually create a new iterator in each recursive call (allOrganizations.iterator() - creates new iterator instance). So when you do call delete you modify the collection for the other iterators and that's why it throws that exception.

One solution would be to use some CopyOnWriteList which allows concurrent modification but it is just passing a copy of the list, not modifying the same one so it will take additional memory.
Another solution would be to add some property to OgOrganizationTreeDTO class that marks if that row is already processed. In this case instead of deleting the item from the list you will just mark it as processed.

But anyway since you are asking about performance and the big O then I can offer you another solution which has O(n) complexity. Of course there is some tradeoff with more memory used but that is the standard memory vs complexity problem...

private static void setChildrenMap(OgOrganizationTreeDTO root, 
     List<OgOrganizationTreeDTO> allOrganizations) {

  Map<Integer, OgOrganizationTreeDTO> organizationsMap = new HashMap<>();
  organizationsMap.put(root.getId(), root);

  for (OgOrganizationTreeDTO element : allOrganizations) {
    organizationsMap.put(element.getId(), element);
  }

  for (OgOrganizationTreeDTO element : allOrganizations) {
     organizationsMap.get(element.getParentId()).addChild(element);
  }

}

Basically taking an element from the hashmap is a constant time so we are using that to find the required parent. You might need to add some checks if you expect elements with bad data because organizationsMap.get(element.getParentId()) might return null

Thank you. Very elegant solution indeed. – Gigaxel Feb 14 '19 at 11:52 — Gigaxel, Feb 14 '19 at 11:52

How to avoid ConcurrentModificationException on recursive iterator based removal from LinkedList?

1 Answers1