I've noticed in the Drill codebase that loggers are being declared static final, but not private. Based on my past experience, this doesn't match common practice.
Loggers should be private: (1) It's common practice. I've never seen otherwise in the past. A quick search turned up these examples in the top search results: - http://www.vogella.com/tutorials/Logging/article.html - http://examples.javacodegeeks.com/core-java/util/logging/java-util-logging-example/ (2) It prevents accidentally using the logger in a derived class. I've been sent off to the wrong place a few times when looking at logs for the Drill code base, because some derived classes have (accidentally?) used their super-classes' loggers. Since the loggers aren't private, this isn't prevented. This gave me the wrong information about where to go looking for messages associated with a problem. (3) Loggers are meant to be associated with exactly one class; that's why they take the class as an argument. Since they're not meant to be used by another class, there's no reason for them to have greater visibility. (There certainly aren't any examples of code that use "OtherClass.logger....") In order to reduce the fragility of code, we reduce the visible surface of classes (aka "encapsulation"), and that means giving everything the least visibility that it requires -- in this case, "private." (4) Private loggers reveal when they aren't being used, at least in IDEs. When they're private, and they aren't used, we can comment them out // private final static Logger logger = ....; and avoid allocating them when we don't need them. If we need them, then we can uncomment them. Does anyone object to making all the loggers in the Drill codebase private from now on? Chris Westin
