One (log.isDebugEnabled()) condition each debug statement occurrence

I would like to che开发者_运维技巧ck with the community, if this is an accepted practice, to have multiple if conditions for each debug statement occurring in a row:

if (log.isDebugEnabled()) log.debug("rproductType = "+ producteType);
if (log.isDebugEnabled()) log.debug("rbundleFlag = " + bundleFrlag);
if (log.isDebugEnabled()) log.debug("rmrktSegment = " + mrktSeegment);
if (log.isDebugEnabled()) log.debug("rchannelTy = " + channelrTy);
if (log.isDebugEnabled()) log.debug("rcompanyPartner = " + coempanyPartner);
if (log.isDebugEnabled()) log.debug("rpreSaleDate = " + preSaleDaete);
if (log.isDebugEnabled()) log.debug("rportNC = " + portrNC);
if (log.isDebugEnabled()) log.debug("rLDC debug end");

I am personally supportive of have a single if condition to wrap up the entire log statements since they are appearing in a row. What are your inputs on this? Or do you see why the original author had wanted to have an if condition for each debug statement?

Thanks!

At best, it is messy. At worst, it performs absolutely redundant function calls.

The only potential difference in logic between sharing ifs is if the debugging option is somehow changed mid-call (possibly by a config reload). But capturing that extra half-a-call really isn't worth the wall of gross code.

Just change it. Don't Repeat Yourself

The reason the if is there at all is to avoid the overhead of building the debug strings if you aren't in debug mode; that part you should keep (Or not keep, if you find this is not a performance critical part of your application).

Edit FYI, by "change it", I mean do this instead:

if (log.isDebugEnabled())
{
  log.debug("rproductType = "+ producteType);
  log.debug("rbundleFlag = " + bundleFrlag);
  // etc
}

The if condition is in there for increased speed.

It is intended to avoid the computational cost of the disabled debug statements. That is, if you have your log level set to ERROR, then there is no need to create the actual message.

http://logging.apache.org/log4j/1.2/apidocs/org/apache/log4j/Category.html

My personal feeling is that the

if  (log.isDebugEnabled())

statments are a micro-optimization that just makes the code harder to read.

I know this was once a best practice for log4j, but many of those checks are performed prior the log staments themselves ( you can see in the source code ) and the only operations you are saving yourself is the string concatenations. I would remove them.

I would do it like that:

if (log.isDebugEnabled()) {
  StringBuilder builder = new StringBuilder();
  builder.append("rproductType = ");
  builder.append(producteType);
  builder.append("rbundleFlag = ");
  builder.append(bundleFrlag);
  builder.append("rproductType = ");
  builder.append(mrktSeegment);
  builder.append("rchannelTy = ");
  builder.append(channelrTy);
  builder.append("rcompanyPartner = ");
  builder.append(coempanyPartner);
  builder.append("rpreSaleDate = ");
  builder.append(preSaleDaete);
  builder.append("rportNC = ");
  builder.append(portrNC);
  builder.append("rLDC debug end");
  log.debug(builder.toString());
}

You have only 2 isDebugEnabled checks in this code: one at the beginning, one in log.debug. The first one prevents the creation of the builder and several short living objects (you heap will thank you)

The second one is obsolet in this case. But it is an pretty easy check, so I think the cost the builder would be higher. Given the fact that in most production system the debug level is off, I think this is the best option.

To summarize, I use isDebugEnabled when my debug statement - or anything else what I need to do for my debug message - becomes more complex then usual. In most cases this is when it comes to String concatenations.

Never use isDebugEnabled for single line log statements. As it has been already mentioned. the log.debug method calls it itself.

Cheers Christian

Depends. Log4j does this very check at the beginning of every method—isDebugEnabled() before debug statement, isWarnEnabled() before warn, etc.,—by default.

This does not mean checks are not required. Checks can save procssing if any of the parameters passed invoke computation. E.g., LOGGER.debug(transformVars(a, b, c, d)); would result in unnecessary execution of transform(), if debug is not enabled!