CodeSOD: An Error on Logging

The beauty of a good logging system is that it allows you to spam logging messages all through your code, but then set the logging level at runtime, so that you have fine grained control over how much logging there is. You can turn the dial from, "things are running smooth in production, so be quiet," to "WTF THINGS ARE ON FIRE GODS HELP US WHAT IS GOING ON CAN I LAUNCH A DEBUGGER ON THE PRODUCTION ENVIRONMENT PLEASE GOD".

You might write something like this, for example:

LOG.error("Error generating file {}", getFileName(), e);

This leverages the logging framework- if error logging is enabled, the message gets logged, otherwise the message is dropped. The string is autoformatted, replacing the {} with the results of getFileName().

That's the code Graham wrote. Graham replaced this code, from another programmer who maybe didn't fully grasp what they were doing:

if (LOG.isErrorEnabled()) { LOG.error(String.format("Generating " + getFileName() + " :%s", e));}

There's a pile of poorly understood things happening here. As stated, if isErrorEnabled is false, LOG.error doesn't do anything. It also can handle string formatting, which isn't what happens here, and what does happen here demonstrates a complete misunderstanding of what String.format does, as it both formats and concatenates.

Then Graham searched through that other programmer's commits, only to find this basic pattern copy/pasted in everywhere, usually with inconsistent indentation. At least it was actual logging, and not a pile of System.out.printlns.

As soon as that though popped into his head, Graham searched again. There were a lot of System.out.printlns too.

[Advertisement] Utilize BuildMaster to release your software with confidence, at the pace your business demands. Download today!