tags:

views:

83

answers:

3

I have some code which is a standalone java application comprising of 30+ classes.

Most of these inherit from some other base classes.

Each and every class has this method to get and use a log4j logger

public static Logger getLogger() {
    if (logger != null) return logger;
    try {
        PropertyUtil propUtil = PropertyUtil.getInstance("app-log.properties");
        if (propUtil != null && propUtil.getProperties() != null)
            PropertyConfigurator.configure(propUtil.getProperties ());
        logger = Logger.getLogger(ExtractData.class);
        return logger;
    } catch (Exception exception) {
        exception.printStackTrace();
    }
}

A) My question is whether this should be refactored to some common logger which is initialized once and used across by all classes? Is that a better practice?

B) If yes, how can this be done ? How can I pass the logger around ?

C) This is actually being used in the code not as Logger.debug() but getLogger().debug(). What is the impact of this in terms of performance?

A: 

Each class can get it's own logger as

final static private Logger log = Logger.getLogger("com.company.package.MyClass");

This enables you to tweak the log4j settings by class, which is more flexible than the standard logger.

glowcoder
+5  A: 

A) in Log4J, you have logger hierarchies, not a single logger. This typically boils down to one logger per class, where loggers are identified by classname. The logger is initialized like this:

private static final Logger logger = Logger.getLogger(MyClass.class);

This is a good practice in that it allows you to fine-tune logging behaviour for modules (packages) or even individual classes within your app. So you may disable logging for some packages, log at INFO level in others, and log on DEBUG level for some critical classes e.g. when you want to catch a bug.

B) However, if you want a single logger everywhere, simply use the root logger in every class:

private static final Logger logger = Logger.getLogger();

C) for calls to not too complex methods, the performance difference is likely negligible, as the JIT compiler will agressively inline calls anyway. For complicated methods it is more of an open question.

Note that the method you show does unnecessary work by loading logger configuration - this is done automatically by Log4J if you name your config file log4j.properties and put it on the classpath. However, even if you need to use a nonstandard config file name, you could still load the Log4J configuration in a single place upon startup, and omit the lazy loading of the logger. Then what is left is

private static final Logger logger = Logger.getLogger(ExtractData.class);

private static Logger getLogger() {
    return logger;
}

and this will surely be inlined by the compiler. You may want to retain getLogger nevertheless, to avoid modifying a lot of caller code.

Note that it is unnecessary for getLogger to be public, as all classes are supposed to have their own logger reference anyway.

Péter Török
+3  A: 

A) Depends, but I think the granularity of per-class loggers is part of the utility of using log4j. You can redirect log information from one class if you want to or send them all to the same file.

B) You wouldn't pass a logger around, you would just use the Logger.getLogger() method to use the root logger.

C) Probably not big in terms of performance. If it wasn't already static, an option would be to indicate your getLogger() method as final to suggest that it should be inlined. Peter's answer is better here, and as he notes, if the method is simple enough, it will likely be inlined anyway.

Jonathon
re C: getLogger() is already static, so making it final would be redundant.
Mike Baranczak
@Mike thanks, you're right.
Jonathon
Declaring a (non static) method `final` does not matter much anyway - the JIT compiler _knows_ whether or not it is overridden in the actual context, so it can inline accordingly.
Péter Török