Can Logger be protected?

[Abu Nene]

Normally we do this to instantiate logger:

public class A {

 private static Logger logger = LoggerFactory.getLogger(A.class);

 ...

}

Is there any concern if I do this:

public abstract class B {

 protected static Logger logger;

 public B() {
  logger = LoggerFactory.getLogger(this.getClass());
 }

 ...

}

public class C extends B {

 public C() {
  super();
 }

 public static void test() {
  logger.debug("test");
 }

 ...

}

[Paul Clapham]

Well, I wouldn't normally write code in which the constructor of a class sets a static variable like that. But apart from that, if the code does what you want it to do then there's nothing wrong with it.

[Paul Beckett]

I wouldn't implement like that. It will probably work if you only have one class extending your abstract class B but as soon as you introduce another class you will get confusing results.

The logger will always be linked to the latest instance created. Try the following code. It is unlikely to give your desired results.

public class D extends C{} 

public class Test{
   public static void main(String... args){
      C c = new C();
      c.test(); //log ouput probably ok

      D d = new D();
      d.test(); //log output probably ok

      c.test(); //log ouput linked to the logger created from D.class. Confusing?!?!
   }
}

 

[Paul Beckett]

btw you test method could throw a nasty NullPointerException. You've declared it static so you don't need an instance of the class but the logger attribute will only be created after you have created an instance of a B.

[Rob Spoor]

How about this:

protected final static Logger logger = LoggerFactory.getLogger(B.class);

The logger gets created once the class is loaded, and can never change. It also takes away the confusion that Paul's class described - the logger is always using class B for logging.

[Abu Nene]

Paul Beckett wrote:btw you test method could throw a nasty NullPointerException. You've declared it static so you don't need an instance of the class but the logger attribute will only be created after you have created an instance of a B.

Hi Paul, your points are valid. Guess I'll stick to instantiate the logger using the private modifier. Thanks!

[Abu Nene]

Rob Prime wrote:How about this:

protected final static Logger logger = LoggerFactory.getLogger(B.class);

The logger gets created once the class is loaded, and can never change. It also takes away the confusion that Paul's class described - the logger is always using class B for logging.

Hi Rob, what about class C? By doing that you are only instantiating the logger with class B as the parameter.

[Rob Spoor]

Class C will use the logger from class B.

If you want each class to have its own logger, then you have two options:
1) make the logger an instance field, not a static field. Each instance has its own logger. You will need instances to log though
2) give each class its own static logger; don't use B's logger in class C at all

As said before by Paul, if you share the logger field among all classes, then C will change B's logger, so B will start logging for class C. That's definitely not what you want.