drill-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Chris Westin <chriswesti...@gmail.com>
Subject Loggers should be private
Date Mon, 02 Mar 2015 19:28:13 GMT
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

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