Add ability for a faster and simpler instation of graphite, leveragin… #1
Conversation
…g sensible defaults.
| @@ -13,6 +13,12 @@ public GraphiteMetricsReporterFactory(GraphiteConfiguration configuration, IGrap | |||
| _graphiteSwitch = graphiteSwitch; | |||
| } | |||
|
|
|||
| public GraphiteMetricsReporterFactory(string ipAddress, string prefix = "" , bool reportMachineName = false, IGraphiteSwitch graphiteSwitch = null) | |||
There was a problem hiding this comment.
Why not add default values to GraphiteConfiguration object?
We are using an object to keep the API backword compatible
There was a problem hiding this comment.
the UX and discoverability of doing it that way is way higher than building an entire object just to discover what config parameters i need, especially since most of the times for simple cases we just need to supply ipAddress.
i accept that the GraphiteConfiguration is a good solution when needed. but it doesn't mean it has to be the only solution.
WDYT?
There was a problem hiding this comment.
I think that using a constructor will lead to a more complex configuration at the long run, since it breaks the encapsulation of the configuratoin.
Whats so painful in creating an object (the default values can be initiated in his ctor the same why), and it would keep backward compatibility, which is the main issue here
There was a problem hiding this comment.
the pain is in lack of simplicity and discoverability.
lack of simplicity - it just more code and more mental leaps to create a new graphite reporter.
discoverability - because from the config class i cannot understand with ease what is a must and what is not.
regarding, "breaks the encapsulation of the configuration", i don't understand what it means? can you describe a sensible scenario in which something will "break".
There was a problem hiding this comment.
"breaks the encapsulation of the configuration" - when you add a parameter to configuration, you'll need to add it to this ctor as well - i.e. breaks encapsulation.
What if we change the design of the configuration object?
Instead of it including all the parameters, it should only include the optional ones.
In the case of GraphiteConfiguration, change it to a ctor which gets the IP (the only actual mandatory parameter), and the rest under GraphiteOptions.
It will make the initial configuration easier, with the flexibility of an object for options.
I've seen this pattern in different libraries (MongoDB.Driver is one of them). It is very convenient.
…g sensible defaults.