Hacker News Reader

Comment by sjroot | Hacker News Reader

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-...

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.

View on HN

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.

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

> 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.

Agree. As much as possible, always leave it up to your caller on whether or not you want the blocking calls to run concurrently. Over-use of `go` as well as unnecessarily buffered channels are two really common anti-patterns with new Go developers.

And don't get me wrong, they're not breaking or absolutely terrible design choices, it's just that they're choices you typically wouldn't make after spending quality time writing apps with the language.

Agreed! In my situation, lots of developers were porting Java APIs to go. They essentially had to be rewritten twice:

1st way: the way our Java devs thought Go was “supposed” to be written

2nd way: how Go should probably actually be written

By embracing the simplicity and Go’s opinionatedness the first time around, you’ll save yourself a lot of refactoring headache later.

> If your enum itself is not a string value, but you NEED a string representation, then it should just implement `Stringer`.

I don't understand. Isn't this exactly what they did?

The only actual problem I see with this enum implementation is that the module-level value->string map is module public, which should definitely be fixed. Other than that, this looks like a bog-standard enum implementation with slightly odd internal naming conventions.

> I seriously have to add it to three different places?

Well, come on, now. The idiomatic example has two places to edit as well, and the third place here is for functionality the idiomatic example doesn't have (mapping back from string to value). You could always generate this map at runtime in an init() function but I suspect some would complain about that as well.

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?

I typically use strings for enum values as well.

> 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.

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

iota/const type enums are covered in another section: 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.

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.

it looks like the generated code of a protobuf enum

Reply on news.ycombinator.com