Our company just started a sonar farm. I was curious about our code quality and wanted to improve.
I have a code containing such logger calls:
LOGGER.error(String.format("Cannot load object in status %s (%s)", status, statusDescription), e);
LOGGER.info(String.format("%s object(s) loaded in status %s (%s)", objects.size(), status, statusDescription));
Sonar triggers rule squid:S262, Invoke method(s) only conditionally "Preconditions" and logging arguments should not require evaluation. Rule triggered by both theses lines.
About this one, I'm not really sure to understand what's going on. The explanation seams not well fitting to my use case. Sonar doc provided this example:
logger.log(Level.DEBUG, "Something went wrong: " + message); // Noncompliant; string concatenation performed even when log level too high to show DEBUG messages
which I perfectly understand (debug will not be logged in production, thus unnecessary operations will occurs). But for info and error level, I would assume that you want to log it anyway. Moreover, in my case, I want both to be logged.
Which is the good approach ? Rewrite differently not using String.format ? Tune sonar to not fire on info/error level ? Just ignore sonar on this one ? Something else ?
Your concatenation will be done before the condition check in the logger. So if you call the logger 10 times and the evaluation returns false, your strings will be concatenated 10 times too with no reason. The logger will handle all concatenation and formatting after it's evaluation is passed and it needs to print something, that way you're saving up on useless operations.
LOGGER.error("Cannot load object in status {} ({})", status, statusDescription, e);
If you love us? You can donate to us via Paypal or buy me a coffee so we can maintain and grow! Thank you!
Donate Us With