4 ms·

This is generally great advice, and aside from the package manager recommendations is still relevant today. I do take issue with a few things though. 1. Don't

by bbatha 9y ago

This is generally great advice, and aside from the package manager recommendations is still relevant today. I do take issue with a few things though. 1. Don't put your internal libraries in /pkg. /pkg has special GOPATH meaning as "compilation cache". Its not an actual name conflict, but why bother risking it. 2. This is just wrong: > fmt.Printf is self-contained and doesn’t affect or depend on global state; in functional terms, it has something like referential transparency. So it is not a dependency. Obviously, f.Bar is a dependency. And, interestingly, log.Printf acts on a package-global logger object, it’s just obscured behind the free function Printf. So it, too, is a dependency. stdout (and the buffer, and mutex on it) are exactly the same as the global log object. In fact, `log.Printf` is more or less just an alias to `fmt.Printf`[0] 3. I wish it had mentioned the functional options pattern in the part about constructors[1] 0: https://github.com/golang/go/blob/b77aad089176ecab971d3a72f0357ed0abc81f4a/src/log/log.go#L294 https://github.com/golang/go/blob/b77aad089176ecab971d3a72f0... 1: https://dave.cheney.net/2014/10/17/functional-options-for-friendly-apis https://dave.cheney.net/2014/10/17/functional-options-for-fr...

kylequest 9y ago
> 1. Don't put your internal libraries in /pkg. /pkg has special GOPATH meaning as "compilation cache". Its not an actual name conflict, but why bother risking it. It's a little too late for this because it's pretty much an unofficial standard for library code :-) Just look at Kubernetes, Docker or many other major Go-based applications/tools.
- lobster_johnson 9y ago
  Last I checked, pkg is going away as a compilation cache. As of Go 1.10, the compilation is in fact now stored elsewhere: https://golang.org/doc/go1.10 https://golang.org/doc/go1.10
atombender 9y ago
> the functional options pattern Functional options are controversial in the Go community. I don't like them personally. The idea of having a bunch of functions that exist only to mutate internal state on init is... an odd choice. Google code is riddled with this sort of style. It's annoying to write client code for another reason: It's hard to discover. For the "term" example in the article, you can't sit in your editor/IDE and type "term." and get a list of setter suggestions, since everything in a single namespace. "term.Speed()" does not sound like an option to me. Function names are generally verbs, and having one called Speed() doesn't read well. And it causes issues if you want to have a type called Speed. Then it has to be term.WithSpeed() or term.SetSpeed() or something. Neither of which is really self-explanatory. The fact that options are functions has other downsides. The moment you pack a option into a function, they've lost their ability to be introspected. You can't dump the options to a debug log before you invoke the constructor function. You can never ask an option function what it contains. I prefer representing options using actual data: type ( Option interface { isOption() } Speed int RawMode bool SomeOtherSetting struct { A, B int } ) Unfortunately, due to how Go interfaces work, you'll have to provide a dummy private method to tie the types together, but it's a small loss. (Go itself uses this pattern in a bunch of places, such as the compiler's AST.)
- chimeracoder 9y ago
  > Functional options are controversial in the Go community. I don't like them personally. The idea of having a bunch of functions that exist only to mutate internal state on init is... an odd choice. Google code is riddled with this sort of style. I come from a background in functional programming, and having used functional options in Go for a while now, I completely agree. I've never seen an implementation of them that felt natural or idiomatic to use. They're incredibly awkward, and I basically always prefer the alternative approaches to the functional options style.
- discoursism 9y ago
  My biggest object to the functional options argument is the step where NewServer(..., Config{}) is considered bad for some reason. As far as I can tell, the entire purported utility of the thing is based on avoiding this obvious approach.
  - lobster_johnson 9y ago
    A big config struct is great, but it comes with a big downside: You have to rely on Go's zero value semantics to provide defaults. It's not always clear that 0 or an empty string means "no value". It means that sometimes you'll have to use dummy values (-1 for "no value", for example) or pointers (nil means optional). It means that any boolean has to be phrased in such a way that false is the default (so if it feels natural to call the option UseKeychain and it should default to true, you have to invert it and call it DontUseKeychain). Config structs also means it's easier for a caller to construct invalid configs: type Config struct { // Either stream or channel can be set, but not both OutputStream io.Writer OutputChan chan Event } // ... pipelines.New(pipelines.Config{ OutputStream: myFile, OutputChan: myChan, }) The semantics cannot be guaranteed by the type system, so won't be caught until runtime. Neither will this, of course: // ... pipelines.New( pipelines.OutputStream(myFile), pipelines.OutputChan(myChan)) ...but in this case we can quite sensibly mandate that the following option overrides the previous one, flipping the internal switch over to the setting you specify. With the Config struct, there's no way for pipelines.New() to know which one should have precedence. (In this particular case, you can work around it by having a single Output field that uses an interface, but there are other situations where mutually incompatible fields can occur.)
pokstad 9y ago
I think `fmt.Printf` is a typo. Immediately preceding this is a code sample referencing `fmt.Fprintf` (which is correct).

by bbatha 9y ago

kylequest 9y ago

> 1. Don't put your internal libraries in /pkg. /pkg has special GOPATH meaning as "compilation cache". Its not an actual name conflict, but why bother risking it. It's a little too late for this because it's pretty much an unofficial standard for library code :-) Just look at Kubernetes, Docker or many other major Go-based applications/tools.

lobster_johnson 9y ago
Last I checked, pkg is going away as a compilation cache. As of Go 1.10, the compilation is in fact now stored elsewhere: https://golang.org/doc/go1.10 https://golang.org/doc/go1.10

atombender 9y ago

> the functional options pattern Functional options are controversial in the Go community. I don't like them personally. The idea of having a bunch of functions that exist only to mutate internal state on init is... an odd choice. Google code is riddled with this sort of style. It's annoying to write client code for another reason: It's hard to discover. For the "term" example in the article, you can't sit in your editor/IDE and type "term." and get a list of setter suggestions, since everything in a single namespace. "term.Speed()" does not sound like an option to me. Function names are generally verbs, and having one called Speed() doesn't read well. And it causes issues if you want to have a type called Speed. Then it has to be term.WithSpeed() or term.SetSpeed() or something. Neither of which is really self-explanatory. The fact that options are functions has other downsides. The moment you pack a option into a function, they've lost their ability to be introspected. You can't dump the options to a debug log before you invoke the constructor function. You can never ask an option function what it contains. I prefer representing options using actual data: type ( Option interface { isOption() } Speed int RawMode bool SomeOtherSetting struct { A, B int } ) Unfortunately, due to how Go interfaces work, you'll have to provide a dummy private method to tie the types together, but it's a small loss. (Go itself uses this pattern in a bunch of places, such as the compiler's AST.)

chimeracoder 9y ago
> Functional options are controversial in the Go community. I don't like them personally. The idea of having a bunch of functions that exist only to mutate internal state on init is... an odd choice. Google code is riddled with this sort of style. I come from a background in functional programming, and having used functional options in Go for a while now, I completely agree. I've never seen an implementation of them that felt natural or idiomatic to use. They're incredibly awkward, and I basically always prefer the alternative approaches to the functional options style.
discoursism 9y ago
My biggest object to the functional options argument is the step where NewServer(..., Config{}) is considered bad for some reason. As far as I can tell, the entire purported utility of the thing is based on avoiding this obvious approach.
- lobster_johnson 9y ago
  A big config struct is great, but it comes with a big downside: You have to rely on Go's zero value semantics to provide defaults. It's not always clear that 0 or an empty string means "no value". It means that sometimes you'll have to use dummy values (-1 for "no value", for example) or pointers (nil means optional). It means that any boolean has to be phrased in such a way that false is the default (so if it feels natural to call the option UseKeychain and it should default to true, you have to invert it and call it DontUseKeychain). Config structs also means it's easier for a caller to construct invalid configs: type Config struct { // Either stream or channel can be set, but not both OutputStream io.Writer OutputChan chan Event } // ... pipelines.New(pipelines.Config{ OutputStream: myFile, OutputChan: myChan, }) The semantics cannot be guaranteed by the type system, so won't be caught until runtime. Neither will this, of course: // ... pipelines.New( pipelines.OutputStream(myFile), pipelines.OutputChan(myChan)) ...but in this case we can quite sensibly mandate that the following option overrides the previous one, flipping the internal switch over to the setting you specify. With the Config struct, there's no way for pipelines.New() to know which one should have precedence. (In this particular case, you can work around it by having a single Output field that uses an interface, but there are other situations where mutually incompatible fields can occur.)

pokstad 9y ago

I think `fmt.Printf` is a typo. Immediately preceding this is a code sample referencing `fmt.Fprintf` (which is correct).