drill-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Steven Phillips <sphill...@maprtech.com>
Subject Re: Loggers should be private
Date Tue, 03 Mar 2015 04:03:01 GMT
I have no objection to making the loggers private, and your reasons make
sense to me. But I do wonder if there was a reason for not making them
private to begin with.

On Mon, Mar 2, 2015 at 12:27 PM, Aditya <adi@apache.org> wrote:

> Sounds good to me.
>
> On Mon, Mar 2, 2015 at 11:28 AM, Chris Westin <chriswestin42@gmail.com>
> 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
> >
>



-- 
 Steven Phillips
 Software Engineer

 mapr.com

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