3 ms·

I don't write a huge amount of Go, but why not something like: conf := sarama.WithDefaults(Config{ ClientID: "foo", RackID: "bar" }) To prevent accidenta

I don't write a huge amount of Go, but why not something like: conf := sarama.WithDefaults(Config{ ClientID: "foo", RackID: "bar" }) To prevent accidental use, add a "Verified" field that must be set before the configuration is accepted. You could also use the above style in a single shot with NewConsumer if you didn't need to retain the configured struct after creation.

everforward 4y ago
The problem isn't that users are accidentally using it. People are using it intentionally. The problem is that some of the config fields are mandatory, and some are optional, but the developer can't tell whether the fields were intentionally set to a weird value or just not specified, and users can't tell which fields need to be set to not have strange behavior. You would need to add a Verified field for each config option so you can tell whether users set it or it's a default. Alternately, you can use pointers, but that gets clunky because you can't do conf := sarama.Connect(Config{ ClientID: &"foo"}) You have to declare a variable above, and then reference the var. Zero values are sometimes valid config options, but not what most people actually want to do. Structs cause issues because there's no way to specify just a couple of fields in the struct; the rest will get zero values.
nemothekid 4y ago
The problem with your solution is: 1. If the verified field is not set, your program blows up at runtime. I don't think it's acceptable for programs to blow up at runtime for errors that can be caught at compile time. 2. Your example still has the same problem when it comes to `MaxMessageBytes`, what if I really meant to set it to 0? The `WithDefaults` function would eat that. You could use a pointer, but then that just complicates the API (you need to do new(int) as a separate variable and assign it

everforward 4y ago

The problem isn't that users are accidentally using it. People are using it intentionally. The problem is that some of the config fields are mandatory, and some are optional, but the developer can't tell whether the fields were intentionally set to a weird value or just not specified, and users can't tell which fields need to be set to not have strange behavior. You would need to add a Verified field for each config option so you can tell whether users set it or it's a default. Alternately, you can use pointers, but that gets clunky because you can't do conf := sarama.Connect(Config{ ClientID: &"foo"}) You have to declare a variable above, and then reference the var. Zero values are sometimes valid config options, but not what most people actually want to do. Structs cause issues because there's no way to specify just a couple of fields in the struct; the rest will get zero values.

nemothekid 4y ago

The problem with your solution is: 1. If the verified field is not set, your program blows up at runtime. I don't think it's acceptable for programs to blow up at runtime for errors that can be caught at compile time. 2. Your example still has the same problem when it comes to `MaxMessageBytes`, what if I really meant to set it to 0? The `WithDefaults` function would eat that. You could use a pointer, but then that just complicates the API (you need to do new(int) as a separate variable and assign it