[ 
https://issues.apache.org/jira/browse/KAFKA-943?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13698401#comment-13698401
 ] 

Jay Kreps commented on KAFKA-943:
---------------------------------

Hey Sam, thanks for the patch it is great to get clean-up patches.

I have sort of a philosophical disagreement, though. I am likely the cause of 
the direct use of string names. This was done intentionally, here is the 
rationale:

Programmers have kind of a knee-jerk belief that using strings directly is bad 
and having variables that point at the string is good. However I don't agree. 
Here is what I think. A global variable is a layer of indirection. By having a 
layer of indirection you get the following trade-off: it is harder to figure 
out which configuration is pointed to by the variable BUT it is easier to 
change the configuration string later and have all uses updated. That is 
normally a good trade-off to make--we don't want string constants floating 
around all over the place. However configuration names are different--these are 
a sort of public api to everyone who uses our system. There are hundreds or 
thousands of people with these strings in their configuration files or code or 
whatever. Adding a layer of indirection doesn't help these people at all. So 
the fact is that configurations are meant to be a public api of sorts and only 
change as part of a major refactoring. So in other words we don't want to make 
it easy. At the same time if we use a variable name instead of the 
configuration name people think less about the name and we end up with bad 
names, which is really irritating.

Our approach to this was to get all the committers to call out and ask for 
review any configuration names in their JIRA and try to get extra review on 
these and then not change them. 

Let me know if you buy that argument. I don't think it is a huge deal either 
way, but I am a little on the side of using the name directly.
                
> Move all configuration key string to constants
> ----------------------------------------------
>
>                 Key: KAFKA-943
>                 URL: https://issues.apache.org/jira/browse/KAFKA-943
>             Project: Kafka
>          Issue Type: Improvement
>          Components: config
>    Affects Versions: 0.8
>            Reporter: Sam Meder
>         Attachments: configConstants.patch
>
>
> The current code base has configuration key strings duplicated all over the 
> place. They show up in the actual *Config classes, a lot of tests, command 
> line utilities and other examples. This makes changes hard and error prone. 
> DRY...
> The attached patch moves these configuration keys to constants and replaces 
> their usage with a reference to the constant. It also cleans up a few old 
> properties and a few misconfigured tests. I've admittedly not written a whole 
> lot of Scala, so there may be some improvements that can be made, in 
> particular I am not sure I chose the best strategy for keys needed by the 
> SyncProducerConfigShared trait (or traits in general).

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira

Reply via email to