2

When I add logging (e.g. log4j2 in Java) to existing code, is it ok (good practice), to alter the code?

Consider the following example:

 //if process returned 0 return true
 return (returnCode == process.waitFor());

This construction is rather neat, but when I want to add logging to it, and I want to use two different levels of logging (DEBUG vs ERROR) depending on the process return code, it can change to something like this:

int returnCode = process.waitFor();

if(returnCode == 0)
{
    logger.debug("Command returned code 0");
}
else
{
    logger.error("Command returned code: ");
}
return (returnCode == 0);

Another example would be:

...
throw new UnsupportedOperationException("Unknown message: " + message.toString());

changed to:

String errorMsg = "Unknown message: " + message.toString();
logger.error(errorMsg);
throw new UnsupportedOperationException(errorMsg);

Is altering the code in order to add logs acceptable? Are there some guidelines on how to do it?

  • 1
    At a PPOE one of the coding standards was that return statements could only return simple values, and not the result of a function call or comparison. This made adding logging statements easier (as well as debugging). – TMN Mar 18 '16 at 12:46
5

Broadly speaking, yes, of course you can alter the code to add logging. If logging had been part of the project from the beginning, how would the code have been written?

There's exceptions to every rule, but these are so situation dependent it's out-of-scope to discuss here. (Consider a critical production problem where logging must be added to diagnose the issue. It's probably prudent to wait until the problem is resolved before making changes to the code beyond the addition of the necessary logging).

In the first example, I would probably add a logProcessResult(...) function to prevent having to look at that if/else construct all over the place just for logging purposes.

The second example is more complicated... it's not always the case that the exception and log should contain the same content, especially if the exception can be publicly visible while the log is not. However, separating the string allows for localization of the message, so isn't a bad idea anyway.

-1

Best practice for handling logging kind of cross cutting concerns through AspectJ/AOP and handle the input parameter and results logging from single place. This shall enhance the code maintainability.

  • 3
    Nothing indicates that the OP wants to log only at the boundaries of the method and not inside the business logic itself. While AOP has benefits, I find it unfortunate to see it mentioned for anything related to logging, as it was a silver bullet. – Arseni Mourzenko Mar 18 '16 at 12:41
  • Question initiated with return, existing code base along with modification as constraint and enhancing maintainability as objective lead to suggest AOP and as you rightly pointed surely it is not as silver bullet. – Venkat Govindan Mar 31 '16 at 11:33

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.