What do you think about returning the logging sentence

Question

I have this java code:

if(isNull(office) || isNull(pricelist)) {
    log.warn("The document {0}-{1} is not valid.",codDoc,numDoc);
    return null;
}

Do you think it is ok if I rewrite it as:

if(isNull(office) || isNull(pricelist)) 
    return log.warn("The document {0}-{1} is not valid.",codDoc,numDoc);

That way

public void warn(String logLine, Object... args)
{...}

would become:

public Object warn(String logLine, Object... args)
{...;return null;}

Take into consideration that logging is mandatory in this system. Thanks for your comments.

Answer 1

It depends what you need the logging to do.

If your system needs to have an object be created like an exception during the logging process, the second way (Object warn()) might work for you.

It really depends on what you need....

Answer 2

If you're always going to return null from the warn method, then it wouldn't make any difference either way; no additional functionality is enabled and no additional information is provided. I would only perform the change if some external entity demands that your logging method match a signature where Object is returned.

Answer 3

I do not think it's OK to rewrite it as you've suggested.

Couple of reasons.

1. Many standard log APIs have void return type for warn method. For somebody familiar with these APIs it would be a surprizing side-effect
2. Your warn method does not return a real object, it always returns null. This would be surprizing for maintainer to find this out.

Answer 4

Use the first method.

The reason being that the second method implies that the return value from the warn method is meaningful. The second method, as written, forces the return value from the warn method to appear as if it were the return value from the caller of the warn method and this is probably the wrong thing to return, especially if the design has various return values to handle various warning conditions.

Answer 5

I would never do that as it creates a surprising API. In Java especially, the point is not tersness, the point is clarity. Having an extra line for the return is valuable in that it tells you the return isn't related to the logging.

And if you are in a method that itself has a void return you would have to put the return on a seperate line anyway.

And anyway, if you declare it to return Object, the method has to return Object, or else you have to cast it down, which gets even worse. You could work on that with generics:

  public <T> T warn(String message, Object... params) { return null; }

But none of this seems to be a good idea in the general case.