Rolf Engelhard

Recently I refactored a legacy application which based on a self-implemented logging-framework (I replaced the proprietary logging-code with slf4j). Along that I stumbled over a bunch of anti-patterns I want to discuss further on.

1. Logger-instantiation inside a method

In general there should be only one logger per class. Most of the common logging-frameworks are caching their loggers after instantiation. So calling a LoggerFactory inside a method is simply an unnecessary code and a (slight) performance waste. Just instantiate a static final logger in the class and use that one on every logging event.

Anti-pattern

public void foo() {
	...
	Logger logger = LoggerFactory.getLogger(Foo.class);
	logger.info("bar!");
	...
}

Pattern

private final static Logger LOGGER = LoggerFactory.getLogger(Foo.class);

public void foo() {
	...
	LOGGER.info("Bar");
	...
}

2. Logger-instantiation using the exception message

This is one of my favorites: on each exception a new logger is created using the exception message as logger name. That neither makes sense nor does it help when analyzing the log-files. It also wastes resources: if the exception messages differ, every factory-call will create a new logger instance (instead of returning a cached one) and additionally adds the new logger to the factory’s cache. As a result the cache will be blown up with multiple logger instances—which won’t be used furthermore.

Anti-pattern

public void foo() {
	try {
	...
	} catch (Exception e) {
		Logger logger = LoggerFactory.getLogger(e.getMessage());
		logger.error(e.getMessage(), e);
	}
}

Pattern

private final static Logger LOGGER = LoggerFactory.getLogger(Foo.class);

public void foo() {
	try {
		...
	} catch (Exception e) {
		LOGGER.error(e.getMessage(), e);
	}
}

3. Logger-instantiation using the enclosing class’ name as string

For ordinary loggers the factory-method of LoggerFactory should be called passing the enclosing class. Avoid to pass a class’ name as string. If you use the enclosing class, your favorite IDE will automatically suit that occurrence on a change of the class’ name (if you’re using vi or something similar to scourge yourself—at last your compiler tells you about it). Your logger will always be related to the current class’ name, a string will not give you that guarantee.

Anti-pattern

privat final static Logger LOGGER = LoggerFactory.getLogger("de.bar.Foo");

Pattern

private final static Logger LOGGER = LoggerFactory.getLogger(Foo.class);

4. Logger-instantiation somewhere in between

A specialization of a common basic: define your class constants and attributes at the beginning of a class, not somewhere in between [1]. That significantly increases the readability and maintainability of your code. Apparently that also holds for loggers.

Anti-pattern

public class Foo {
	// ... code ...
	private final static Logger LOGGER = LoggerFactory.getLogger(Foo.class);
	// ... even more code ...
}

Pattern

public class Foo {

	private final static Logger LOGGER = LoggerFactory.getLogger(Foo.class);

	// ... code
}