search

gofrs / uuid

A UUID package for Go
MIT License
1.55k stars 108 forks source link

Mixed pointer and value recivers #130

Closed andreykyz closed 5 months ago

andreykyz commented 5 months ago

https://github.com/gofrs/uuid/blob/22c52c268bc0dcc0569793f5b1433db423f5a9c6/uuid.go#L286C1-L286C36 according to https://go.dev/tour/methods/8

In general, all methods on a given type should have either value or pointer receivers, but not a mixture of both. (We'll see why over the next few pages.)

zerkms commented 5 months ago

https://go.dev/tour is a beginners tutorial to the language. Does it cause any demonstrable issues?

dylan-bourque commented 5 months ago

Does it cause any demonstrable issues?

I'll second this point. Much of the code in this repo came from github.com/satori/go.uuid so we (the current maintainers) aren't fully responsible for the decisions that were made there. I'd argue against making a purely cosmetic change for this.

FWIW, I'd support following this "rule" for any new code.

andreykyz commented 5 months ago

Does it cause any demonstrable issues?

Pointer receiver means that instance can be nil instead of value receiver which cause segfault in that situation.

andreykyz commented 5 months ago

FWIW, I'd support following this "rule" for any new code.

There are much better UUID library.