triemap icon indicating copy to clipboard operation
triemap copied to clipboard

Fix `updateValue` input

Open alexeyraspopov opened this issue 7 years ago • 2 comments

I may be missing some context here, but it seems to be a correct assumption based on what updateValue() does.

alexeyraspopov avatar Sep 04 '17 12:09 alexeyraspopov

@norswap, any chance to get this reviewed? Thanks

alexeyraspopov avatar Apr 17 '18 21:04 alexeyraspopov

I apologize, I didn't notice this or forgot it existed.

I gave it a quick glance and something seems wrong to me: v1 is the old value, and v is the new value. As such, it seems the correct code (going from the comment in Change and the body of updateValue) is neither my original nor yours, but rather:

change.replaced(v1)
return updateValue(bit, v)

It would also explain why the implementation passes the test, since I don't test the Change object.

Does this make sense or am I missing something?

norswap avatar Apr 20 '18 13:04 norswap