Iterator removal/edit is safe, however I also need to edit all following Iterations

Question

I have read a little about ConcurrentModificationException in stackflow and my actual update appears not to be the issue, it could be a problem in my design or I need a technique I haven't learnt yet.

Example Situation: My iterator is running along position markers. Then an action can be performed to shift the markers over (e.g. Inserting into string). All Markers greater than the current position must also be shifted to preserve correctness.

Task: How do I update the remaining markers without the iterator exploding? Can I refresh the iterator, or break and start the loop again?

The following code is abstracted from my work.

 public void innerLoop(Boolean b) {
    //An Example of what I'm working with
    HashMap<String, HashSet<Integer>> map = new HashMap<String, HashSet<Integer>>() {
        {
            put("Nonce",
                new HashSet<Integer>() {

                {
                    add(1);
                    add(2);
                    add(3);
                    add(4);
                    add(5);
                }
            });
        }
    };

    //for each key
    for (String key: map.keySet()) {
        HashSet<Integer> positions = map.get(key);

        //for each integer
        for (Iterator<Integer> it = positions.iterator(); it.hasNext();) {
            Integer position = it.next();

            System.out.println("position =" + position);
            //(out of scope) decision requiring elements from the outter loops
            if (new Random().nextBoolean()&&b) {
                //shift position by +4 (or whatever)
                //and every other (int >= position)
                System.out.println("Shift " + position + " by 4");
                Integer shift = 4;
                update(position,
                       shift,
                       positions);
                it.remove();
            }
        }
    }
}

public void update(Integer current,
                   Integer diff,
                   Set<Integer> set) {

    if (set != null) {
        HashSet<Integer> temp = new HashSet<Integer>();
        for (Integer old: set) {
            if (old >= current) {
                temp.add(old);
                System.out.println(old + "Added to temp");
            }
        }

        for (Integer old: temp) {
            set.remove(old);
            System.out.println(old + "removed");
            set.add(old + diff);
            System.out.println((old + diff) + "Added");
        }
    }
}

Edited with Garrett Hall Solution

 public void nestedloops() {

    HashMap<String, HashSet<Integer>> map = new HashMap<String, HashSet<Integer>>() {
        {
            put("Hello",
                new HashSet<Integer>() {

                {
                    add(5);
                    add(2);
                    add(3);
                    add(4);
                    add(1);
                    add(6);
                }
            });
        }
    };

    //for each key
    for (String key: map.keySet()) {
        ArrayList<Integer> positions = new ArrayList<Integer>(map.get(key));
        //for each integer
        for (int i = 0; i < positions.size(); i++) {
            Integer position = positions.get(i);
            System.out.println("[" + i + "] =" + position);
            //out of scope decision
            if (new Random().nextBoolean()) {
                //shift position by +4
                //and every other (int >= position)
                System.out.println("Shift after " + position + " by 4");
                Integer shift = 4;
                //Update the array
                for (int j = 0; j < positions.size(); j++) {
                    Integer checkPosition = positions.get(j);
                    if (checkPosition > position) {
                        System.out.println(checkPosition + "increased by 4");
                        positions.set(j,
                                      checkPosition + shift);
                    }
                }
            }
        }
        //Add updated Array
        map.put(key,
                new HashSet<Integer>(positions));
    }
}

Garrett Hall · Accepted Answer · 2013-10-17T15:05:25.280

1

You best bet is indexing the HashSet by putting it into a list. Then you can use indices to refer to elements rather than an Iterator. So long as you are not removing or adding (only updating) elements, then your indices will be correct. Otherwise you will have to account for that. Example:

ArrayList<Integer> positions = new ArrayList<Integer>(map.get(key));
for (int i = 0; i < positions.size(); i ++) {
  // updating list
  for (int j = i; i < positions.size(); j ++) {
    positions.set(j, positions.get(i) + diff);
  }
}

edited Oct 17 '13 at 15:05

answered Oct 17 '13 at 13:58

Garrett Hall

29,524
10
61
76

map.keyset() returns Set I would need `for(String key: map.keyset() ){ ArrayList positions = new ArrayList(map.get(key));` So I move along the array of ints but I can change the values as long as I don't change the size of the array? Initially that looks like what I want, let me check – Another Compiler Error Oct 17 '13 at 14:50
The `ArrayList` should be instantiated the same way `positions` is in your current code. (Update with `map.get(key)` to reflect that). – Garrett Hall Oct 17 '13 at 15:06
@GarrettHall I have added you solution to my question. – Another Compiler Error Oct 17 '13 at 15:51
@GarrettHall is the put I placed at the end required to retain the changes made? – Another Compiler Error Oct 17 '13 at 15:55
Correct, ultimately you need to modify `map` it seems. – Garrett Hall Oct 17 '13 at 16:34

score 0 · Answer 2 · edited May 23 '17 at 12:21

0

I would copy the original set to a list so that you don't need to worry about the current iteration code. Then update a secondary list (not being iterated).

Reasons:

You can't iterate and modify your original collection at once (there is no way around the ConcurrentModificationExceptions)

Nice one liner to shift items in a list.

Collections.rotate(list.subList(j, k+1), -1);

Guava will be able to handle the "find first index that satisfies that predicate and transform the list" which a bunch of utility methods.

edited May 23 '17 at 12:21

Community

1
1

answered Oct 17 '13 at 14:21

Anthony Accioly

21,918
9
70
118

Q: If I have "editPositions" and "loopingPositions". In loopingPositions I find I want to shift `position = 3`. I update `editPositions = {1, 2, 3+4, 4+4, 5+4} = {1,2,7,8,9}` the next iteration of loopingPositions will be `position = 4`, which is out-dated when I really needed `position = 8`. Is that what would happen? – Another Compiler Error Oct 17 '13 at 14:37
Well, nothing stops you from swapping the original list for it's copy at each Iteration, them produce a new copy, rotate it, swap with the previous copy and so on (it is expensive but will work). Views can also mitigate performance problems. – Anthony Accioly Oct 17 '13 at 14:50
A better strategy would involve a cumulative counter of "rotated positions" (that starts with `0`), then you always consider `loopingPosition + cumulativeCounter` as the position to shift `editPositions`, and, at the end of each iteration, make `cumulativeCounter += diff` this way you can iterate over the original values and still get the desired result using only a single copy of the original collection. – Anthony Accioly Oct 17 '13 at 14:58
I've added your edit to the question as best I can understand. It appears to work correctly but how do I retain on exit the appropriate amount of shift to per position. `{1+(4*0), 2+(4*1), 3+(4*2), 4+(4*2), 5(4*3)}` = `{1,6,11,12,17}` – Another Compiler Error Oct 17 '13 at 16:26
Just create a temp `List` and add `positionValue + counterShift` (after `counterShift` has been updated) at each iteration... Or use `Guava` [transform](https://code.google.com/p/guava-libraries/wiki/FunctionalExplained#Functions) functions to do it in one step ;) – Anthony Accioly Oct 17 '13 at 16:44

Iterator removal/edit is safe, however I also need to edit all following Iterations

2 Answers2