#413 Cyclomatic Complexity picks up isDebugEnabled conditionals

open
nobody
Check (274)
1
2012-10-10
2006-02-21
Michael
No

The Cyclomatic Complexity rule counts isDebugEnabled()
conditionals in its calculation which results in false
measurements since these conditionals do not really
result in branches in the code that need to be tested.
As a result, on our project we have lots of false
warnings, so many that we must either raise the limit
(with the risk that really complex methods may slip by)
or deactivate the rule altogether.

A simple fix would allow the exclusion of conditionals
based on isDebugEnabled.

Discussion

  • Oliver Burn
    Oliver Burn
    2006-02-21

    Logged In: YES
    user_id=218824

    I can see why you have requested this feature, but as is,
    this request is very specific to your needs if the check is
    based on calling the method isDebugEnabled(). Other projects
    would use other mechanisms, so it would be required to be
    configurable.

    Personally I do not recommend littering the code with
    isDebugEnabled() style guards as I find it to be premature
    optimisation of the code. Instead I add the guard only where
    necessary for performance reasons.

     
  • Michael
    Michael
    2006-02-21

    Logged In: YES
    user_id=655163

    I used to feel exactly the same way as you, I'd only put
    isDebugEnabled in critical sections of code. But on my last
    project we had a hard time with performance and I spent
    weeks optimizing with JProfiler. I saw how much of a
    difference it makes. Our entire app's performance was
    killed by a single debug statement in a method called for
    every query to the backend. Getting it fixed required a fix
    from another team, a delivery, etc. Now I see now reason
    not to add it, in order to avoid all that. In fact we have
    Jalopy configured to automatically put the debug guards in
    for us.

    Anyway, I think it's comment enough to justify having an
    option to not count this. It could be generic with a
    regular expression though to be more flexible. I'm sure
    this is a common problem, people either don't use the debug
    guards or they turn the check off. I don't see any other
    way around it.

     
  • Michael
    Michael
    2006-06-23

    Logged In: YES
    user_id=655163

    It's been 4 months, I'm curious if there are any news for
    this? Any plans to implement it? I'm currently finding
    myself auditing a project and I notice their CC metrics are
    through the roof. I look closely at their code and they
    have hundreds of debug statements. Their debug statements
    are completely screwing up the metrics. So now I don't have
    a useful metric on the CC..

    Note PMD has a bug open for the same problem, been 4 months
    too..

     
  • Lars Kühne
    Lars Kühne
    2006-06-24

    Logged In: YES
    user_id=401384

    I am not aware of anyone working on this, so if you really
    need this you might want to start implementing it yourself,
    as your own private plugin.

    Patches that implement this feature in a generic way would
    be welcome. However, that is pretty difficult to achieve:

    • People use different logging frameworks, i.e. different
      isDebugEnabled methods (your regexp idea might work to
      handle this).
    • People use local variables to cache the value of
      isDebugEnabled()
    • log4j has recently introduced a trace level (below debug),
      so you need to handle that as well. Same goes for
      java.util.logging FINE FINER FINEST
    • People might include more control flow logic inside the if
      block, and you will want to control whether that adds to
      cylomatic complexity or not. Example:

    if (logger.isDebugEnabled()) {
    if (someOtherCondition) {
    logger.debug("A");
    } else {
    logger.debug("B");
    }
    }