search

hashicorp / go-set

The go-set package provides generic Set implementations for Go, including HashSet for types with a Hash() function and TreeSet for orderable data
Mozilla Public License 2.0
118 stars 8 forks source link

Why isn't `Set` recommended to be used with pointers? #70

Closed efulmo closed 1 year ago

efulmo commented 1 year ago

Documentation for set.New() says

T must not be of pointer type, nor contain pointer fields, which are comparable but not in the way you expect. For these types, use HashSet instead.

It's a confusing statement. I use Set with addresses to a struct that has some pointer fields and it works just fine - it keeps unique pointers in the set. As far I understand from the source code, Set has no limitations for pointers or structs with pointers, it just may work in unobvious way for unexperienced developers that don't understand difference between address comparison and deep struct comparison. README explains this aspect better.

shoenig commented 1 year ago

Hi @efulmo, you're correct in that a Set can be used with pointers or structs containing pointers as long as the pointer comparability is what you're actually expecting. When I wrote that doc string I was thinking about our main use case, where it's common to use a .Equal method for comparing equality of complex structs, where the behavior of pointer comparisons is wrong.

e.g. https://play.golang.com/p/PPD110sXh9h

So yeah, the doc should be tweaked to be more accurate like the README. Would you be willing to open up a PR?

shoenig commented 1 year ago

Closed by #74