Some method calls can effectively be "no-ops", meaning that the invoked method does nothing, based on the application’s configuration (eg: debug
logs in production). However, even if the method effectively does nothing, its arguments may still need to evaluated before the method is called.
Passing message arguments that require further evaluation into a Guava com.google.common.base.Preconditions check can result in a
performance penalty. That is because whether or not they’re needed, each argument must be resolved before the method is actually called.
Similarly, passing concatenated strings into a logging method can also incur a needless performance hit because the concatenation will be performed
every time the method is called, whether or not the log level is low enough to show the message.
Instead, you should structure your code to pass static or pre-computed values into Preconditions conditions check and logging
calls.
Specifically, the built-in string formatting should be used instead of string concatenation, and if the message is the result of a method call,
then Preconditions should be skipped altogether, and the relevant exception should be conditionally thrown instead.
Noncompliant code example
logger.log(Level.DEBUG, "Something went wrong: " + message); // Noncompliant; string concatenation performed even when log level too high to show DEBUG messages
logger.fine("An exception occurred with message: " + message); // Noncompliant
LOG.error("Unable to open file " + csvPath, e); // Noncompliant
Preconditions.checkState(a > 0, "Arg must be positive, but got " + a); // Noncompliant. String concatenation performed even when a > 0
Preconditions.checkState(condition, formatMessage()); // Noncompliant. formatMessage() invoked regardless of condition
Preconditions.checkState(condition, "message: %s", formatMessage()); // Noncompliant
Compliant solution
logger.log(Level.DEBUG, "Something went wrong: {0} ", message); // String formatting only applied if needed
logger.log(Level.SEVERE, () -> "Something went wrong: " + message); // since Java 8, we can use Supplier , which will be evaluated lazily
logger.fine("An exception occurred with message: {}", message); // SLF4J, Log4j
LOG.error("Unable to open file {0}", csvPath, e);
if (LOG.isDebugEnabled()) {
LOG.debug("Unable to open file " + csvPath, e); // this is compliant, because it will not evaluate if log level is above debug.
}
Preconditions.checkState(arg > 0, "Arg must be positive, but got %d", a); // String formatting only applied if needed
if (!condition) {
throw new IllegalStateException(formatMessage()); // formatMessage() only invoked conditionally
}
if (!condition) {
throw new IllegalStateException("message: " + formatMessage());
}
Exceptions
catch blocks are ignored, because the performance penalty is unimportant on exceptional paths (catch block should not be a part of
standard program flow). Getters are ignored as well as methods called on annotations which can be considered as getters. This rule accounts for
explicit test-level testing with SLF4J methods isXXXEnabled and ignores the bodies of such if statements.
