ManevilleF / hexx

Hexagonal tools lib in rust
Apache License 2.0
288 stars 23 forks source link

Add Ord and PartialOrd to Hex #146

Closed targrub closed 7 months ago

targrub commented 8 months ago

Motivation: To allow use of std::cmp with Vec. https://rust-lang.github.io/api-guidelines/interoperability.html#c-common-traits

ManevilleF commented 8 months ago

Could you expand on the use case it might solve? Also, what would the comparison do? Compare the vector lengths ?

alice-i-cecile commented 8 months ago

Ord is a surprisingly useful trait on weird types, as it allows for them to be stored in data structures like BTreeSet.

ManevilleF commented 8 months ago

Ord is a surprisingly useful trait on weird types, as it allows for them to be stored in data structures like BTreeSet.

Agreed but there are multiple valid comparisons:

And choosing one seems like the wrong way to go, there could be wrappers like OrdByXY(Hex), OrdByYX(Hex) and OrdByLength(Hex) implementing both PartialOrd and Ord

targrub commented 7 months ago

My use case was just to be able to test whether two Vec<Hex>s were equal or not.

ManevilleF commented 7 months ago

No need of Ord for that, unless you need sorting. But then I would advise sorting manually by one the three options. If you share a code snippet I can help you with that

targrub commented 7 months ago

Was user error. Withdrawing PR. But I do have confusion about this guideline: https://rust-lang.github.io/api-guidelines/interoperability.html#c-common-traits