drill-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Aditya <...@apache.org>
Subject Re: Loggers should be private
Date Mon, 02 Mar 2015 20:27:00 GMT
Sounds good to me.

On Mon, Mar 2, 2015 at 11:28 AM, Chris Westin <chriswestin42@gmail.com>

> 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

  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message