7 ms·

Warning: opinions inbound I clicked one part (enums) and noticed a pretty glaring issue. The way you are doing enums is really _not_ conventional Go code and p

by sjroot 6y ago

Warning: opinions inbound I clicked one part (enums) and noticed a pretty glaring issue. The way you are doing enums is really _not_ conventional Go code and probably not something I would allow past a PR review. I’m saying this as someone who has built many production systems with Go and taught it to many devs coming from Java. This guide is more in-line with how Go enums should be designed: https://blog.learngoprogramming.com/golang-const-type-enums-iota-bc4befd096d3 https://blog.learngoprogramming.com/golang-const-type-enums-... So, while I dig a little further, I think it’s fair to warn people to take this with a grain of salt. Use the language yourself and come to your own conclusions as OP did.

colesantiago 6y ago
> So, while I dig a little further, I think it’s fair to warn people to take this with a grain of salt. Use the language yourself and come to your own conclusions as OP did. I'm sure with the experience you have with Go, you can open a pull request to their notes to fix this issue.
- sjroot 6y ago
  If no one has by EOD today, I will! Good idea. :)
arwineap 6y ago
I use this pattern without iota; tending towards constants as strings. This allows it to "serialize" to strings nicely instead of exposing useless (without context) integers Do you have any thoughts on iota vs strings?
- deleted 6y ago
  [deleted]
- sjroot 6y ago
  I typically use strings for enum values as well.
curryst 6y ago
What is the advantage of building out enums your way vs OPs? I've done both ways, and I didn't find any situations where one was demonstrably better than the other. Is it that there are no globals? Is it that it's actually constant? One potential annoyance to me in your design is that there isn't an easily accessible map of enum values and their string representation. That is really useful for testing; most of my enums end up with a .Validate() method that checks whether the underlying value is actually a valid member of the enum. If I have that map, it's trivial to right a test that iterates over enum keys and ensure that a) valid keys pass Validate(), and b) enum.String() returns the corret string. Enums (theoretically) shouldn't be modified at runtime, so it should be perfectly safe and valid to have a single, global instance of that data. I thought the use of maps to store the enum keys/values was odd, but when I thought about it, maps allow the enum keys to be sparse, which can be valuable.
- sjroot 6y ago
  First off, I should clarify that I'd actually love to see Go add support for proper enums, as I've certainly found it lacking. My biggest issues would be maintainability and, as you mentioned, using global maps. If your enum itself is not a string value, but you NEED a string representation, then it should just implement `Stringer`. As far as maintainability: - The naming convention used here is not consistent with how enums are named in Go. For a good example, see the HTTP package, specifically request methods and response status codes [1]. Really, it isn't even an appropriate naming convention for constant values in any language, with `UPPERCASE_UPPERCASE_minor_note`, it looks a bit silly and took longer for me to grok. - What if I want to add a new enum variant? I seriously have to add it to three different places? If I start touching this map in places outside the original source file, things will get really ugly really fast. - A bit nit-picky, but: why use an `int32` here? This file, in my opinion, is over-engineered. Go code should be simple; many people learning Go for the first time hesitate to work that way, unfortunately. Other common issues I see with people learning Go for the first time: - Overuse of concurrency. People like to use the `go` statement wherever possible, and create overly-complex APIs with channels. Unless you have multiple events that need to be done in parallel, yet simultaneously needing to communicate between them, you should not use them in your package's public API. Let your package consumers choose when to make that call. - Package structure, particularly in modules. People like `src` directories, but Go doesn't work that way. Your top priority as someone learning Go should be to dig through Go's standard library itself and see how it is organized. [1] https://golang.org/pkg/net/http/#pkg-constants https://golang.org/pkg/net/http/#pkg-constants
randomdata 6y ago
iota/const type enums are covered in another section: https://github.com/betty200744/ultimate-go/blob/master/Language_Specification/constant/constant.go https://github.com/betty200744/ultimate-go/blob/master/Langu... The code in the section labeled Enums looks like it is straight out of the Protobuf compiler. grpc is referenced later on, so that structure pattern may be related.
- sjroot 6y ago
  Ah, that would make sense. But then I'd have to dip into my jar of opinions on gRPC + Protobufs... TLDR: Before going down that route, you should make sure you really REALLY need them. Someone learning Go for the first time probably doesn't.
deleted 6y ago
[deleted]
ggordan 6y ago
it looks like the generated code of a protobuf enum

by sjroot 6y ago

colesantiago 6y ago

> So, while I dig a little further, I think it’s fair to warn people to take this with a grain of salt. Use the language yourself and come to your own conclusions as OP did. I'm sure with the experience you have with Go, you can open a pull request to their notes to fix this issue.

sjroot 6y ago
If no one has by EOD today, I will! Good idea. :)

arwineap 6y ago

I use this pattern without iota; tending towards constants as strings. This allows it to "serialize" to strings nicely instead of exposing useless (without context) integers Do you have any thoughts on iota vs strings?

deleted 6y ago
[deleted]
sjroot 6y ago
I typically use strings for enum values as well.

curryst 6y ago

What is the advantage of building out enums your way vs OPs? I've done both ways, and I didn't find any situations where one was demonstrably better than the other. Is it that there are no globals? Is it that it's actually constant? One potential annoyance to me in your design is that there isn't an easily accessible map of enum values and their string representation. That is really useful for testing; most of my enums end up with a .Validate() method that checks whether the underlying value is actually a valid member of the enum. If I have that map, it's trivial to right a test that iterates over enum keys and ensure that a) valid keys pass Validate(), and b) enum.String() returns the corret string. Enums (theoretically) shouldn't be modified at runtime, so it should be perfectly safe and valid to have a single, global instance of that data. I thought the use of maps to store the enum keys/values was odd, but when I thought about it, maps allow the enum keys to be sparse, which can be valuable.

sjroot 6y ago
First off, I should clarify that I'd actually love to see Go add support for proper enums, as I've certainly found it lacking. My biggest issues would be maintainability and, as you mentioned, using global maps. If your enum itself is not a string value, but you NEED a string representation, then it should just implement `Stringer`. As far as maintainability: - The naming convention used here is not consistent with how enums are named in Go. For a good example, see the HTTP package, specifically request methods and response status codes [1]. Really, it isn't even an appropriate naming convention for constant values in any language, with `UPPERCASE_UPPERCASE_minor_note`, it looks a bit silly and took longer for me to grok. - What if I want to add a new enum variant? I seriously have to add it to three different places? If I start touching this map in places outside the original source file, things will get really ugly really fast. - A bit nit-picky, but: why use an `int32` here? This file, in my opinion, is over-engineered. Go code should be simple; many people learning Go for the first time hesitate to work that way, unfortunately. Other common issues I see with people learning Go for the first time: - Overuse of concurrency. People like to use the `go` statement wherever possible, and create overly-complex APIs with channels. Unless you have multiple events that need to be done in parallel, yet simultaneously needing to communicate between them, you should not use them in your package's public API. Let your package consumers choose when to make that call. - Package structure, particularly in modules. People like `src` directories, but Go doesn't work that way. Your top priority as someone learning Go should be to dig through Go's standard library itself and see how it is organized. [1] https://golang.org/pkg/net/http/#pkg-constants https://golang.org/pkg/net/http/#pkg-constants

randomdata 6y ago

iota/const type enums are covered in another section: https://github.com/betty200744/ultimate-go/blob/master/Language_Specification/constant/constant.go https://github.com/betty200744/ultimate-go/blob/master/Langu... The code in the section labeled Enums looks like it is straight out of the Protobuf compiler. grpc is referenced later on, so that structure pattern may be related.

sjroot 6y ago
Ah, that would make sense. But then I'd have to dip into my jar of opinions on gRPC + Protobufs... TLDR: Before going down that route, you should make sure you really REALLY need them. Someone learning Go for the first time probably doesn't.

deleted 6y ago

[deleted]

ggordan 6y ago

it looks like the generated code of a protobuf enum