I was wondering what shenanigans I can get up to with streams and mutating the underlying collection, and what a terrible idea this is.
As we all know, it's a really bad idea to mutate a list on which you are iterating, and Java makes certain this fails with a ConcurrentModificationException.
It all starts with the original problem, a for-loop that also mutates the list on which is iterates:
@Test(expectedExceptions = ConcurrentModificationException.class) public void simpleTest() { List<Long> longs = new ArrayList<>(List.of(12L, 13L, 16L, 20L, 55L, -5L, 12L, 5L, 100L, 1000L)); for (Long aLong : longs) { if (aLong > 50L || aLong < 0L) { longs.remove(aLong); } } assertThat(longs).isEqualTo(List.of(12L, 13L, 16L, 20L, 12L, 5L)); }
Nowadays, we can use the forEach method on a collection, turning it into something like this with exactly the same problem:
@Test(expectedExceptions = ConcurrentModificationException.class) public void forEachTest() { List<Long> longs = new ArrayList<>(List.of(12L, 13L, 16L, 20L, 55L, -5L, 12L, 5L, 100L, 1000L)); longs.forEach(t -> { if (t > 50L || t < 0L) { longs.remove(t); } }); assertThat(longs).isEqualTo(List.of(12L, 13L, 16L, 20L, 12L, 5L)); }
If you are using a stream, the same problem occurs, as the stream refers to the underlying collection:
@Test(expectedExceptions = ConcurrentModificationException.class) public void streamTest() { List<Long> longs = new ArrayList<>(List.of(12L, 13L, 16L, 20L, 55L, -5L, 12L, 5L, 100L, 1000L)); longs.stream() .filter(Objects::nonNull) .filter(t -> t > 50L || t < 0L) .forEach(longs::remove); assertThat(longs).isEqualTo(List.of(12L, 13L, 16L, 20L, 12L, 5L)); }
One way, though admittedly not a great way, to fix this, is to transfer it to a list first. Like so:
@Test public void streamWithToListTest() { List<Long> longs = new ArrayList<>(List.of(12L, 13L, 16L, 20L, 55L, -5L, 12L, 5L, 100L, 1000L)); longs.stream() .filter(t -> t > 50L || t < 0L) .collect(Collectors.toList()) .forEach(longs::remove); assertThat(longs).isEqualTo(List.of(12L, 13L, 16L, 20L, 12L, 5L)); }
But my colleague at work mentioned the new removeIf function, which seems to work particularly well:
@Test public void removeIfTest() { List<Long> longs = new ArrayList<>(List.of(12L, 13L, 16L, 20L, 55L, -5L, 12L, 5L, 100L, 1000L)); longs.removeIf(t -> t > 50L || t < 0L); assertThat(longs).isEqualTo(List.of(12L, 13L, 16L, 20L, 12L, 5L)); }
Of course, the whole thing is patently ridiculous, because you might as well just filter unwanted elements out of the stream and create a new list from it.
But in some cases, where the "remove" is not as simple as chucking an element out of a collection (for example something as involved as, oh, I don't know, ImportantRestService.removeItem(Item item)), one has to find other ways to deal with it, which is where the second-to-last solution might come in handy.
I don't know. I'm open to suggestions.
