The basket kept a set
of the items in it.
Add an item,
it goes in the set.
Change the quantity,
update the item.
Remove it,
take it out of the set.
Simple enough
to pass review without a comment.
Then customers started reporting
that an item they had removed
was still in the basket.
And sometimes
the same item appeared twice.
Nobody could reproduce it
by clicking around.
It only happened
when somebody changed a quantity
before they removed the line.
Here is what a hash set does
when you hand it an object.
It asks the object for its hash code,
and uses that number
to pick a bucket.
The object goes in that bucket
and the set never looks at it again
until you ask.
The item's hash code
was built from its product
and its quantity.
So when the quantity changed,
the object in the bucket
started reporting a different number.
Ask the set
whether it contains the item,
and it works out the new hash,
goes to the new bucket,
finds nothing there,
and says no.
Ask it to remove the item,
same bucket,
same nothing.
Add it again,
and now you have two,
equal to each other,
in two different places.
The object was not lost.
It was exactly where the set put it.
The set was asked to look somewhere else.
The rule is old
and easy to forget.
Anything a hash code is built from
must not change
while the object is a key.
So build the hash
from what makes the thing the thing.
The line id.
Not the quantity.
Not the price.
Not anything a user can edit.
Better still,
make keys immutable.
A record,
or a class with final fields,
so changing a quantity
means making a new line
and the old one is replaced,
not edited where it sits.
If you truly need a mutable object
in a collection,
key the map by its id
and keep the object as the value.
Then write the test
that would have caught it.
Put something in a set.
Change it.
Ask whether it is still there.
A collection remembers
where you put something,
not what it became.
– Serguey Asael Shinder
Top comments (0)