Sounds good to me. On Mon, Mar 2, 2015 at 11:28 AM, Chris Westin <[email protected]> wrote:
> 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 >
