[ 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