SlowerBuffer

Programming and Technology Related Stuff

But Is It in Debug?

Came across this code in one of our applications yesterday:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
   if (LOG.isDebugEnabled())
   {
      if ( LOG.isDebugEnabled() ) {
         LOG.debug(parameter.getLabel() + " ==================================");
      }
      if ( LOG.isDebugEnabled() ) {
         LOG.debug("\tlabel     = " + parameter.getLabel());
      }
      if ( LOG.isDebugEnabled() ) {
         LOG.debug("\tproperty  = " + property);
      }
      if ( LOG.isDebugEnabled() ) {
         LOG.debug("\tdbValue   = " + value);
      }
      if ( LOG.isDebugEnabled() ) {
         LOG.debug("\tbeanValue = " + BeanUtils.getProperty(bean, parameter.getProperty()) + "\n");
      }
   }
   else
      ; // not debugging

I found it rather hilarious. It’s not terrible code, certainly not The Daily WTF worthy (though maybe not exactly my java coding style). I mean, it’s not terribly inefficient as the conditional doesn’t cost much and, besides, the extra checks will only execute when in DEBUG, at which point you’ve got larger performance issues. But I still wanted to know who would have written this?

The answer: Global Find and Replace

I checked the revision history. The file hasn’t been edited since 2006 and prior to that edit it looked like this:

1
2
3
4
5
6
7
8
9
10
   if (LOG.isDebugEnabled())
   {
      LOG.debug(parameter.getLabel() + " ==================================");
      LOG.debug("\tlabel     = " + parameter.getLabel());
      LOG.debug("\tproperty  = " + property);
      LOG.debug("\tdbValue   = " + value);
      LOG.debug("\tbeanValue = " + BeanUtils.getProperty(bean, parameter.getProperty()) + "\n");
   }
   else
      ; // not debugging

The committed was an old employee of ours, and after seeing his name and the date I recalled that he at one point spent some time going through the code looking for instances where debug logging was done without first checking the log level (if you don’t know why this is a good idea, here’s a read). I can only assume that he decided to run a global find and replace for this, as I can’t really see him hand crafting this example.

My point: Be very careful with full code base find and replace. In this case there’s no problem, just some amusingly useless code, but there could easily be a situation where an actual performance issue is introduced and not quickly caught.

Comments