Changing a collection while traversing might lead to unexpected errors

TL;DR: Do not modify collections while traversing them

Problems

• Unexpected Results

• Concurrency problems

Solutions

1. Avoid altering the collections

2. Make collection copies

Context

We over-optimize our solutions with the prejudice that copying collections is expensive.

This is not true for small and medium-size collections.

Languages iterate collections in many different ways.

Modifying them is generally not safe.

Sample Code

Wrong

Collection<Integer> people = new ArrayList<>();
//here we add elements to the collection...

for (Object person : people) {
    if (condition(person)) {
        people.remove(person);
    }
}
//We iterate AND remove elements

Right

Collection<Integer> people = new ArrayList<>();
//here we add elements to the collection...

List<Object> iterationPeople = ImmutableList.copyOf(people);

for (Object person : iterationPeople) {
    if (condition(person)) {
        people.remove(person);
    }
}
//We iterate a copy and remove from original

coll.removeIf(currentIndex -> currentIndex == 5);
//Or use language tools (if available)

Detection

[X] Semi Automatic

Many languages provide control both in compile and run time-

Tags

• Fail Fast

Conclusion

This is something we learn in our first courses.

It happens a lot in the industry and real-world software

Relations

More Info

• Stack Overflow

Credits

Photo by Christine Roy on Unsplash

---

Bugs are bugs. You write code with bugs because you do. If it’s a safe language in the sense of run-time safe, the operating system crashes instead of doing a buffer overflow in a way that’s exploitable.

Ken Thompson

---

This article is part of the CodeSmell Series.