search

kyren / hashlink

An updated version of linked-hash-map and friends
Apache License 2.0
98 stars 18 forks source link

Ordering in equality checks #5

Closed benjaminwinger closed 3 years ago

benjaminwinger commented 3 years ago

LinkedHashSet currently ignores ordering when comparing sets, however LinkedHashMap does not.

I'm aware that this would be an API-breaking change, but wouldn't it make more sense for LinkedHashMap to also ignore ordering when comparing maps?

Or perhaps, if the old behaviour is to be preserved, LinkedHashMap should accept the equality function as a type parameter, which defaults to ordered equality.

kyren commented 3 years ago

I think that the best thing to do would probably be for LinkedHashMap and LinkedHashSet to both ignore ordering when comparing maps to make them drop in replacements for HashMap and HashSet, and to then provide a separate method to compare them that includes ordering.

The extra type parameter is maybe a bit complex for a niche feature, but either that or a type wrapper could work?

kyren commented 3 years ago

Actually I think I've changed my mind, instead of being compatible with HashMap, it should remain compatible with LinkedHashMap from the linked-hash-map crate, I think LinkedHashSet is the implementation that's wrong here. This also retains consistency with the current sensible Ord implementation as well.

Instead let's do the reverse, change then LinkedHashSet to match and if it comes up, add an order independent equality check if anyone wants it.

kyren commented 3 years ago

Addressed in 4fb1b6df94219f0905dcff584c93d9aa558b698d

benjaminwinger commented 3 years ago

Okay. I'd since worked around it anyway, so it's not a pressing issue for me now.