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?

score 123 · Accepted Answer · edited Jan 21 '15 at 08:18

123

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.

edited Jan 21 '15 at 08:18

Dave Jarvis

30,436
41
178
315

answered Aug 28 '12 at 13:44

Peter Lawrey

525,659
79
751
1,130

1

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)? – omnomnom Aug 28 '12 at 13:48
10

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. – Peter Lawrey Aug 28 '12 at 13:51
1

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 – shuniar Aug 28 '12 at 13:55
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. – Emilio Jul 27 '15 at 09:25
1

@Emilio I would call `log()` a getter as all it does it is get a value. What would you use instead of `protected`? – Peter Lawrey Jul 27 '15 at 11:37
@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("..")) – Emilio Jul 27 '15 at 12:07
1

@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. – Peter Lawrey Jul 27 '15 at 12:11
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? – cyotee doge Aug 09 '17 at 17:53
@cyotee `Object.getClass()` will always get the actual class of the instance. – Peter Lawrey Aug 10 '17 at 18:23
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. – rgoers Aug 05 '18 at 16:22
@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. – Peter Lawrey Aug 05 '18 at 17:25
@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. – rgoers Aug 05 '18 at 17:30
@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. – Peter Lawrey Aug 05 '18 at 17:38

score 16 · Answer 2 · edited Oct 26 '18 at 23:42

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");
    }
}

score 5 · Answer 3 · answered Aug 28 '12 at 13:48

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.

score 4 · Answer 4 · answered Jun 28 '17 at 05:05

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.
    }
}

score -1 · Answer 5 · answered Aug 28 '12 at 13:51

-1

If you create the logger in the abstract class, the logs will all come out tagged as originating from AbstractFoo. If you want/need to see logs tagged with the child class from which the log occurred, create loggers for the children classes.

answered Aug 28 '12 at 13:51

herrtim

2,697
1
26
36

"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. – cellepo Sep 08 '16 at 20:19
The 'problem' with Lawrey's answer is that they are now instance based loggers, which isn't idea. – MeBigFatGuy Nov 05 '17 at 14:02
@MeBigFatGuy Is it bad to have instance based loggers? – valijon Apr 20 '18 at 06:46
4

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. – MeBigFatGuy Apr 21 '18 at 12:41

Java Logging With Abstract Classes

5 Answers5

Linked