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.

This is the second part of “Logging Anti-Patterns”.

5. Log and Throw

There’s another well known principle: “Either handle or throw your exception” [1]. Never do both at a time. So if you substitute “handle” with “log” you get “Either log or throw your exception”.

Imagine the following problem when performing the anti-pattern “Log and throw”: if you log your exception and throw it, the calling method might log that exception too. And hey – maybe the next one does the same. Your log-files are going to hold the same information several times. Which doesn’t increase the logs’ readability.

Anti-pattern

public void foo() {
	try {
		...
	} catch (Exception e) {
		LOGGER.error(e.getMessage(), e);
		throw e; // or: throw new WrappingException(e);
	}
}

Pattern

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

public void foo() {
	try {
		...
	} catch (Exception e) {
		throw e; // or: throw new WrappingException(e);
	}
}

6. Withhold the exception

Remember the saying “don’t let bugs get lost without a trace”? If you’re not giving the exception to your logger, you simply throw away information. The exceptions cause and stacktrace are meaningful hints while tracing a bug in your application. So always call the logger including the exception.

Notice that if you do not want to have your stacktrace in the logs – that is something your logger configuration is responsible of, not your business logic.

Anti-pattern

public void foo() {
	try {
	...
	} catch (Exception e) {
		LOGGER.error("An exception occurred."); 
		// or: LOGGER.error(e.getMessage());
	}
}

Pattern

public void foo() {
	try {
		...
	} catch (Exception e) {
		LOGGER.error("An exception occurred.", e); 
		// or: LOGGER.error(e.getMessage(), e);
	}
}

7. Use of System.out.println, System.err.println, e.printStackTrace

Sad but true – a code-smell which doesn’t seem to get exterminated. In the (common) case where your logger uses appenders which don’t only write to the console you lose every information written via System.out, System.err and e.printStackTrace.

For example if your logger writes to a specified log-file and you use System.out.println(“Extremely useful message”), then your “extremely useful message” will never be written to the log-file. It pops out on your console and than – data nirvana.

Anti-pattern

public void foo() {
	try {
		...
	} catch (Exception e) {
		System.out.println(e.getMessage())
		// or: System.err.println(e.getMessage()) 
		// or: e.printStackTrace(); 
	}
}

Pattern

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

8. Not using an abstraction layer

If your application uses an abstraction layer for logging, you can change the concrete logging framework whenever you like—by simply exchanging the related jars and configuration files. There is no necessity to change code. Especially when developing a framework, the consumers will be grateful for this abstraction layer.

With slf4j [2] exists a great and widely used way to do so.

Pattern

// make sure you just import slf4j,
// not log4j, logback, commons-logging ...
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;