5 ms·

> func NewServer(... config *Config ...) http.Handler one of my biggest pet peeves is when people take a Config object, which represents the configuration of a

by zemo 3y ago

> func NewServer(... config *Config ...) http.Handler one of my biggest pet peeves is when people take a Config object, which represents the configuration of an entire system, and pass it around mutably. When you do that, you're coupling everything together through the config object. I've worked on systems where you had to configure the parts in a specific order in order for things to work, because someone decided to write back to the config object when it was passed to them. Or another case was where I've seen it such that you couldn't disable a portion of the system because it wrote data into the config object that was read by some other subsystem later. The pattern of "your configuration is one big value, which is mutable" is one of the more annoying patterns that I've seen before, both in Go and in other languages.

doh 3y ago
I think that's a valid criticism. What do you think would be a more ergonomic pattern?
- Raynos 3y ago
  I wrote a static config class that reads configuration for the entire app / server from a JSON or YAML file ( https://github.com/uber/zanzibar/blob/master/runtime/static_config.go https://github.com/uber/zanzibar/blob/master/runtime/static_... ). Once you've loaded it and mutated it for testing purposes or for copying from ENV vars into the config, you can then freeze it before passing it down to all your app level code. Having this wrapper object that can be frozen and has a `get()` method to read JSON like data make it effectively not mutable.
  - doh 3y ago
    I use similar pattern myself. Was curious if the OP is using some other, like for instance splitting the struct into two (im/mutable) and then passing them around, or what. BTW kudos on zanzibar. Love the tech and the code).
- zemo 3y ago
  I just use a struct literal, and then I have the type define a `func (t *Thing) ready() error { ... }` method and call the ready method to check that its valid. I prefer this over self-referential options, the builder pattern, supplying a secondary config object as a parameter to a constructor, etc.
- gabesullice 3y ago
  Not the OP, but I mitigate the issue rather than use a different pattern. Like so: type Server struct { val bool } type Config struct { Val bool } func NewServer(... config *Config ...) http.Handler { if config == nil { config = &Config{} } return &Server{ val: config.Val } } It took me a long time to settle on this pattern and I admit it's tedious to copy configuration over to the server struct, but I've found that it ends up being the least verbose and maintainable long term while making sure callers can't mutate config after the fact. I can pass nil to NewServer to say "just the usual, please", customize everything, or surgically change a single option. It's also useful for maintaining backwards compatibility. I'm free to refactor config on my server struct and "upgrade" deprecated config arguments inside my NewServer function.
tejinderss 3y ago
The keyword here is “mutable” config object and not config data object in general. I use immutable config dataclass liberally in one of my python projects and i pass it around in all modules. Many functions rely on multiple values and instead of passing all of them as function parameters (which requires their own function typings), the dataclass has all variables with typing definitions in one place, its pretty handy design pattern.
gloryjulio 3y ago
I agree. We ran into sev by changing the top level config object before. You DO NOT want to modify it. The wasted man hour is not worth. You will never know where or how it get used. If you make changes it's better to derive from it instead. Update: What's funny was, in our design the config object was kinda immutable. You have to use the WARNING_DO_NOT_USE api to make modification. We did mutate the object and we caused a sev
fnordlord 3y ago
I've tended to create a Config struct for each package and then a configs.Config struct that's just made up of each package's Config. It might not be a Go best practice but I like that I can setup the entire system's configuration on startup as one entity but then I only pass in the minimally required dependencies for each package. It also makes testing a little easier because I don't have to fake out the entire configuration for testing one package.
MrDarcy 3y ago
My favorite way to prevent this is to make the config truly immutable, but still configurable with something like this: package config type options struct { name string } type Option func(o *options) func Name(name string) Option { return func(o *options) { o.name = name } } type Config struct { opts *options } func New(opts ...Option) *Config { o := &options{} for _, option := range opts { option(o) } return &Config{opts: o} } func (c *Config) Name() string { return c.opts.name } Use it with: cfg := config.New(config.Name("Emanon")) fmt.Println(cfg.Name())
- zemo 3y ago
  I used that pattern for a while but stopped using it. I first encountered it from this blog post: https://commandcenter.blogspot.com/2014/01/self-referential-functions-and-design.html https://commandcenter.blogspot.com/2014/01/self-referential-... It's a lot of boilerplate to create something that's not actually immutable. It also makes it harder to figure out which options are available, since now you can't just look at the documentation of the type, you have to look at the whole module package to figure out what the various options are. If one of the fields is a slice or map you can just mutate that slice or map in place, so it's not really immutable. The pattern as Pike describes it has the benefit that supplying an option returns an option that reverses the effect of supplying the option so that you can use the options somewhat like Python context objects that have enter and exit semantics, but in practice I've found that to be useful in a small portion of situations.
  - kubanczyk 3y ago
    > you can't just look at the documentation of the type Sure you can. Option func is a constructor for option type, and constructors are auto-included above methods in the docs. PLS completion works for them as well.
    - zemo 3y ago
      The options for the thing being constructed are all separate types from the thing being constructed; the options aren’t a facet of the definition of the type they mutate.
lelanthran 3y ago
> one of my biggest pet peeves is when people take a Config object, which represents the configuration of an entire system, and pass it around mutably. How do you create immutable structs in Go? I didn't think you could, which makes this more a Go problem than a `passing around Config object` problem. (One of my pet peeves, coming to Go from C, is how little of stronger typing there actually is. In C, I pass and return const objects everywhere I can, my enums are not just ints because the compiler can warn when I forget one in a switch statement, etc).

by zemo 3y ago

doh 3y ago

I think that's a valid criticism. What do you think would be a more ergonomic pattern?

Raynos 3y ago
I wrote a static config class that reads configuration for the entire app / server from a JSON or YAML file ( https://github.com/uber/zanzibar/blob/master/runtime/static_config.go https://github.com/uber/zanzibar/blob/master/runtime/static_... ). Once you've loaded it and mutated it for testing purposes or for copying from ENV vars into the config, you can then freeze it before passing it down to all your app level code. Having this wrapper object that can be frozen and has a `get()` method to read JSON like data make it effectively not mutable.
- doh 3y ago
  I use similar pattern myself. Was curious if the OP is using some other, like for instance splitting the struct into two (im/mutable) and then passing them around, or what. BTW kudos on zanzibar. Love the tech and the code).
zemo 3y ago
I just use a struct literal, and then I have the type define a `func (t *Thing) ready() error { ... }` method and call the ready method to check that its valid. I prefer this over self-referential options, the builder pattern, supplying a secondary config object as a parameter to a constructor, etc.
gabesullice 3y ago
Not the OP, but I mitigate the issue rather than use a different pattern. Like so: type Server struct { val bool } type Config struct { Val bool } func NewServer(... config *Config ...) http.Handler { if config == nil { config = &Config{} } return &Server{ val: config.Val } } It took me a long time to settle on this pattern and I admit it's tedious to copy configuration over to the server struct, but I've found that it ends up being the least verbose and maintainable long term while making sure callers can't mutate config after the fact. I can pass nil to NewServer to say "just the usual, please", customize everything, or surgically change a single option. It's also useful for maintaining backwards compatibility. I'm free to refactor config on my server struct and "upgrade" deprecated config arguments inside my NewServer function.

tejinderss 3y ago

The keyword here is “mutable” config object and not config data object in general. I use immutable config dataclass liberally in one of my python projects and i pass it around in all modules. Many functions rely on multiple values and instead of passing all of them as function parameters (which requires their own function typings), the dataclass has all variables with typing definitions in one place, its pretty handy design pattern.

gloryjulio 3y ago

I agree. We ran into sev by changing the top level config object before. You DO NOT want to modify it. The wasted man hour is not worth. You will never know where or how it get used. If you make changes it's better to derive from it instead. Update: What's funny was, in our design the config object was kinda immutable. You have to use the WARNING_DO_NOT_USE api to make modification. We did mutate the object and we caused a sev

fnordlord 3y ago

I've tended to create a Config struct for each package and then a configs.Config struct that's just made up of each package's Config. It might not be a Go best practice but I like that I can setup the entire system's configuration on startup as one entity but then I only pass in the minimally required dependencies for each package. It also makes testing a little easier because I don't have to fake out the entire configuration for testing one package.

MrDarcy 3y ago

My favorite way to prevent this is to make the config truly immutable, but still configurable with something like this: package config type options struct { name string } type Option func(o *options) func Name(name string) Option { return func(o *options) { o.name = name } } type Config struct { opts *options } func New(opts ...Option) *Config { o := &options{} for _, option := range opts { option(o) } return &Config{opts: o} } func (c *Config) Name() string { return c.opts.name } Use it with: cfg := config.New(config.Name("Emanon")) fmt.Println(cfg.Name())

zemo 3y ago
I used that pattern for a while but stopped using it. I first encountered it from this blog post: https://commandcenter.blogspot.com/2014/01/self-referential-functions-and-design.html https://commandcenter.blogspot.com/2014/01/self-referential-... It's a lot of boilerplate to create something that's not actually immutable. It also makes it harder to figure out which options are available, since now you can't just look at the documentation of the type, you have to look at the whole module package to figure out what the various options are. If one of the fields is a slice or map you can just mutate that slice or map in place, so it's not really immutable. The pattern as Pike describes it has the benefit that supplying an option returns an option that reverses the effect of supplying the option so that you can use the options somewhat like Python context objects that have enter and exit semantics, but in practice I've found that to be useful in a small portion of situations.
- kubanczyk 3y ago
  > you can't just look at the documentation of the type Sure you can. Option func is a constructor for option type, and constructors are auto-included above methods in the docs. PLS completion works for them as well.
  - zemo 3y ago
    The options for the thing being constructed are all separate types from the thing being constructed; the options aren’t a facet of the definition of the type they mutate.

lelanthran 3y ago

> one of my biggest pet peeves is when people take a Config object, which represents the configuration of an entire system, and pass it around mutably. How do you create immutable structs in Go? I didn't think you could, which makes this more a Go problem than a `passing around Config object` problem. (One of my pet peeves, coming to Go from C, is how little of stronger typing there actually is. In C, I pass and return const objects everywhere I can, my enums are not just ints because the compiler can warn when I forget one in a switch statement, etc).