benbjohnson / immutable

Immutable collections for Go
MIT License
713 stars 32 forks source link

generics: widen map key constraint to 'comparable' #29

Closed laher closed 1 year ago

laher commented 1 year ago

Addressing https://github.com/benbjohnson/immutable/issues/25 (second attempt)

As suggested by @anacrolix after my previous attempt (https://github.com/benbjohnson/immutable/pull/26):

I gave it a spin, it didn't work for the underlying user (although the stm code passed). I think the type can be widened for SortedMap (and any other variants too). Again, because comparers are manually given, the restriction on types belong to those implementations. I'm pretty sure it works for the builtin comparers too, as those use type sets to check for orderable types anyway. Short answer: I think K comparable can be used everywhere in immutable.

So I used comparable, but under the hood it only supports strings, the various integer types, and derived types.

As before, the defaultComparer panics if you try to use any other key type. You just need to write your own Comparer for other types.

benbjohnson commented 1 year ago

@laher Sorry for the late reply. AFAICT this looks ok. @anacrolix what do you think?

anacrolix commented 1 year ago

Give me a few days to try it out.

BarrensZeppelin commented 1 year ago

I've been using this for a while without issues. :+1:

benbjohnson commented 1 year ago

Thanks @laher. I went ahead and merged. If anyone has any objections or changes, go ahead and open a new PR. 👍

anacrolix commented 1 year ago

This did solve the issue, I used it in https://github.com/anacrolix/stm/commit/32174bfc2d877d4b3df131c9e2c7704a82755443 and https://github.com/anacrolix/dht/commit/88ee3382ad4a18a10bfc28476136a97ee094bfd8.

laher commented 1 year ago

Nice one. I just noticed that I didn't update the readme. I can PR it a bit later, and then maybe we need another release?

benbjohnson commented 1 year ago

I went ahead and cut a v0.4.1 release: https://github.com/benbjohnson/immutable/releases/tag/v0.4.1