Junit Testing issue with for loop and return statement

216 views Asked by At

I'm kind of confused with a for-loop & return statements. I hope someone could explain me what's going on.

    private Map<Ticket, Integer> list;

public RessourceBasedAccessControl32603() {
    super();
    list = new HashMap<>();
}

I have to write a working method out of this junit test:

@Test
void testRemoveCollection2() {
    accessControl.add(t1, 10);
    accessControl.add(t2, 20);
    accessControl.add(t3, 30);
    Set<Ticket> s = new HashSet<>();
    s.add(t1);
    s.add(t3);
    int actual = accessControl.remove(s);
    assertEquals(40, actual);
}

If I write this code I get the expected 40 as a result.

    @Override
public int remove(Collection<Ticket> c) { 
    int points = 0;
    for (Ticket i : c) {
        if (list.containsKey(i)) {
            points += list.remove(i);
        }
    }
    return points;
}

If I write this code I get 30 instead of 40. I'm just getting the number of the last add (t3) instead of t1 + t3). I'm pretty sure it's not summing up because of the first return but if I delete the first return I get a "java.util.ConcurrentModificationException". Why can't I just use one return as in the example above? What's the difference? Is there still a way to get the method to sum up all tickets t in the HashSet s?

    @Override
public int remove(Collection<Ticket> c) {
    int points = 0;
    for (Ticket i : list.keySet()) {
        if (c.contains(i)) {
            points += list.remove(i);
            return points;
        }
    }
    return points;
}

Thanks a lot in advance!

2

There are 2 answers

0
XtremeBaumer On BEST ANSWER

The ConcurrentModificationException happens, because you remove an item from the HashMap, while still iterating over it. If you leave the return in, then you don't iterate the key set while modifying it.

If you want to go that route, remove the items after you are done iterating.

public int remove(Collection<Ticket> c) {
    int points = 0;
    for (Ticket i : list.keySet()) {
        if (c.contains(i)) {
            points += list.get(i);
            // return points;
        }
    }
    list.keySet().removeAll(c);
    return points;
}
0
Michał 'PuszekSE' On

In Java you cannot operate on collection using this statement format inside for loop. for (Ticket i : list.keySet()) - internally it's using Iterator from the collection and modifying it during the Iteration breaks the order, thus the exception.

There are two approaches you can use:

First one would be just adding points first and then doing list.removeAll(Collection<?>...).

Second one can be done using older for statement with indexes after transforming the keySet into an actual list like new ArrayList<>(list.keySet())

Loop below uses reversed order to not affect indexing by removing entries within the loop. (Removing index j affects only elements that are at positions greater than j).

for (int i=keyList.length-1; i>=0;i--){
   Ticket t = keyList[i];
   if(c.contains(t)){
      points+=list.remove(t)
   }
}