Java Logging With Abstract Classes

Question

I am working on a project, and am currently working on implementing some logging with log4j and I was curious about how I should go about implementing the logs. The two implementations I am kicking around are as follows:

First Option

Use single log from super class for that class and all sub classes:

public abstract class AbstractFoo {
    protected static Log LOG = LogFactory.getLog(AbstractFoo.class);

    ...
}

public class Foo extends AbstractFoo {
    public void someMethod() {
        LOG.info("Using abstract log");
    }
}

Second Option

Use individual logs for each class, super and subs:

public abstract class AbstractFoo {
    private static Log LOG = LogFactory.getLog(AbstractFoo.class);

    ...
}

public class Foo extends AbstractFoo {
    private static Log LOG = LogFactory.getLog(Foo.class);        

    public void someMethod() {
        LOG.info("Using own log");
    }
}

What makes more sense and why?

Solution 1

I wouldn't do either. Instead I would make it use the correct class in both cases.

public abstract class AbstractFoo {
    protected final Log log = LogFactory.getLog(getClass());

    ...
}

public class Foo extends AbstractFoo {
    public void someMethod() {
        log.info("Using abstract log");
    }
}

If you are not doing lots of logging (which is a good idea anyway) you can use a method instead.

public abstract class AbstractFoo {
    protected Log log() { return LogFactory.getLog(getClass()); }

    ...
}

If there is a class which calls this a lot you can override it to give you a cached instance.

Solution 2

This is my solution (final static logger):

public abstract class AbstractFoo {
     protected abstract Log getLogger();
     public doSomething() {
          getLogger().info("log something");
     }
}

public class Foo extends AbstractFoo {
    private static final Log log = Log.getLogger(Foo.class);

    protected Log getLogger() {
         return log;
    }
    public doSomethingElse() {
          log.info("log somethingElse");
    }
}

Solution 3

Both make sense. It depends on your application.

I think that more often used practice is to have private logger for each class. This allows you to configure logging both per class and per package. Remember, that AbstractFoo and Foo may belong to different packages and probably you want to see logs from Foo only.

Moreover always think twice if you want to write protected field. It is not completely forbidden but a well known bad practice. It makes your code less readable and difficult to maintain.

Solution 4

The same can be achieved by playing with constructors. Add logger at the Base class level and set it from every Derived class using super(). There is the code :

public abstract class AbstractFoo {

    protected Log log;  // base abstract class has a Log object.

    public AbstractFoo(Log logger) {   // parameterized constructor for logger, to be used by the derived class.
        this.log = logger;
    }

    public doSomething() {        // common method for all the derived classes.
      log.info("log something");
    }
    // rest of business logic.
}

public class Foo extends AbstractFoo {

    public Foo(){
        super(LogFactory.getLog(AbstractFoo.class));
    }

    public void someMethod() {
        log.info("Using own log");     // this uses its own logger.
    }
}

View more solutions

Comments

[omnomnom]

So far I've seen two approaches: static loggers (as in the question) and non-static loggers (as in your example). Aren't static loggers better solution (one instance of logger per all instances)?

[Vishy]

static loggers are better if they are the same for all instances. In the abstract class case, the class of the instances are not all the same.

[shuniar]

I like this, it seems like a good way to combine both options. You end up with a single log but it binds to the proper class. +1

[Emilio]

3 years later, I know. This solution seems correct, except for the use of "protected". It is preferable to use a getter in the subclass.

[Vishy]

@Emilio I would call log() a getter as all it does it is get a value. What would you use instead of protected?

[Emilio]

@PeterLawrey protected is a little bit polemic since it breaks the encapsulation principle. I mean, your second solution it's ok, I was talking about first (log.info("Using abstract log"); instead of log().info(".."))

[Vishy]

@Emilio The problem with using log as a field is you have to add code to every sub-class to use the appropriate logger. Although, using a field is much more efficient per call esp if you are not actually logging i.e. logging is off.

[cellepo]

"If you create the logger in the abstract class, the logs will all come out tagged as originating from AbstractFoo" -> Nope, not true if you use the Accepted Answer by @Peter_Lawrey. Then you get logs tagged with class doing the logging, always.

[cyotee doge]

What is the scope of getClass()? In your example it's just there. But in code it's not recognized as it has no scope. I could use this. But how to you ensure you get the subclass?

[Vishy]

@cyotee Object.getClass() will always get the actual class of the instance.

[MeBigFatGuy]

The 'problem' with Lawrey's answer is that they are now instance based loggers, which isn't idea.

[valijon]

@MeBigFatGuy Is it bad to have instance based loggers?

[MeBigFatGuy]

Well you are wasting memory and time allocating these things, and creating more garbage for no reason. There are certainly much more heinous problems in life besides this, but it's kind of dumb.

[rgoers]

While this solution works in the example shown it won't work if you want the logs to reflect AbstractFoo when called from methods in AbstractFoo and Foo when called in methods in Foo. The logger can only have one name.

[Vishy]

@rgoers the solutions assumes that you want to show the actual class rather than the class the method was originally declared in. From the actual class you can determine the implementing class.

[rgoers]

@PeterLawrey Yes, I understand that. But in many cases it is preferable to have the log message tell you what class file to look in rather than look in the subclass and then search each of its ancestors.

[Vishy]

@rgoers this is true, a better solution in that regard is to print the line (class,method,line) in a format which is similar to a stack trace. This allows you to click on the line of text in your IDE and see where in the code it was logged.