LoggerIsNotStaticFinal

Posted on March 27, 2007 by Scott Leberknight

For anyone who uses PMD, the title of this blog appears in their list of PMD errors if they don't declare their loggers static and final. Specifically, the LoggerIsNotStaticFinal rule simply says that a log should be declared static and final. I also like to make sure they are private as well. For example:

// Jakarta Commons Logging
private static final Log log = LogFactory.getLog(MyClass.class);

The above code also shows another good practice, which is to pass the Class object to the getLog() method, instead of a string. Why the java.util.logging.Logger class doesn't even provide a method accepting a Class object is simply beyond me. Why did the people who developed the java.util.logging package base their API on Log4j yet omit some of the most useful parts of it? Oh well.

Now to the point. Why it is good practice to declare loggers private, static, and final? A logger is an internal implementation detail, so it should be private. You only need one logger for all instances of a class, hence static. And a logger should not be able to be replaced, thus final. So if this is good, what's not so good (at least in my opinion)? Simple - any logger that is not private, static, final, and which doesn't pass in a Class object to getLog()! For example, consider this common bit of code, declared in some base class:

// Not so good logger declaration
protected final Log log = LogFactory.getLog(getClass());

Why is this bad? Well, it isn't static for one thing. For another, it uses getClass() to obtain the log. At first this seems efficient since now all subclasses automatically inherit a ready-made log of the correct runtime type. So what's the issue here? The biggest problem with loggers declared in this manner is that you now get all the logging from the superclass mixed in with the logging from the subclass, and it is impossible in the log output to discern which messages came from which class unless you look at the source. This is really annoying if the superclass has a lot of logging that you don't want to see, since you cannot filter it out.

Another problem is that your ability to set log levels differently goes away, for example if a subclass resides in a different package than the superclass. In that case, if you try to filter out logging from the superclass, you can't because the actual runtime class was used to obtain the logger.

Last, having a protected logger just seems to violate basic object-oriented principles. Why in the world should subclasses know about an internal implementation detail from a superclass that is a cross-cutting concern, no less? Anyway, though this is a silly little rant it really is annoying when you extend a superclass that declares a protected logger like this.



Post a Comment:
Comments are closed for this entry.