drill-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Jacques Nadeau <jacq...@apache.org>
Subject Re: Loggers should be private
Date Tue, 03 Mar 2015 04:41:37 GMT
It's a dumb answer but there was a reason. I think they should be available
in every class so I add them to the my new class template. If they are
private you get an unused exception unless you suppress the warning. Making
them package private means no warning and no suppression required.
On Mar 2, 2015 8:03 PM, "Steven Phillips" <sphillips@maprtech.com> wrote:

> 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