LilasCorner / jlootbox

Other
1 stars 0 forks source link

Remove Offers will skip offers #20

Open JohnTMurphy-NIU opened 2 years ago

JohnTMurphy-NIU commented 2 years ago

The loop in Platform.removeOffers(ArrayList offers) will not remove offers correctly. The reason is that the loop modifies the list while also traversing the list, which is always risky and sometimes a no-no. In this case assume that the elements in the list are initially labeled Alpha, Beta, Gamma, Delta, Epsilon, Zeta, and the two that should be removed are Gamma and Delta. The index variable (i) starts at 0, and then goes through 1 and reaches the point where the index (i) = 2, and is pointing at Gamma. It asks if Gamma should be removed, and then removes it. Now the list contains Alpha, Beta, Delta, Epsilon, Zeta; the index is currently 3, which is now pointing at Delta. Then the index is incremented by the loop, so that it becomes 4, and now points at Epsilon, having never asked if Delta should be removed. The final list is Alpha, Beta, Delta, Epsilon, Zeta, which is incorrect.

This is a common issue; one strategy that avoids the problem is to use an index value that starts at list length - 1 and decrements instead of incrementing; in the example above, it would start with 5 and check Zeta, then go to 4 and check Epsilon, then 3 and check Delta, which it would remove; then it decrements to 2 and checks Gamma, which it also removes. Another typical strategy is to only increment the index if the element is not removed; a variation on this (kind of ugly) is that if the element is removed, decrement the index, so that when it is incremented again by the 'for' loop it comes out correct. A final strategy is to not remove elements during the traversal of the loop, but to add the elements to be removed into a separate collection, and after the loop is traversed perform a 'removeAll(toBeRemoved)' on the original collection.

Note also that fancy approaches are also available: https://www.baeldung.com/java-collection-remove-elements