Exclude isolated method from LCOM4 computation

Why would you want to exclude isolated methods? Isn't that exactly what the metric is trying to measure?

In fact no.

If you look at this example, one method throws an UnsupportedOperationException.

If, in your class, you have several of those (perhaps because your class extends another one which has too many abstract methods for your requirements), then it will get an LCOM4 higher than 1.

We took the decision to not be too much conservative in our computation of the LCOM4 metric, to avoid generating false positive. However, if we do report an LCOM4 greater than 1, then it must mean that there is a real underlying issue with the class.

That is why getters and setters are excluded as well.

If we do report an LCOM4 of 2, then the class has 2 distinct and independant groups of methods/fields.

I see your point with the UnsupportedOperationException, but the implementation looks like this:

  
  private void removeIsolatedMethodBlocks() {
    Iterator<Set<AsmResource>> iterator = unrelatedBlocks.iterator();
    
    while (iterator.hasNext()) {
      Set<AsmResource> block = iterator.next();
      if (block.size() == 1) {
        iterator.remove();
      }
    }
    
  }

This means, it effectively ignores all blocks that consist of only one method. This is exactly what LCOM4 tries to measure. Yes, there can be false positives. Yes, you might have to fulfill an contract from a superclass. We can discuss all the details. But this is just too much.

I see what you mean, but I do not think that LCOM4 is used to measure individual methods, but rather groups of unrelated methods/fields within a class.

The thing is that, before we made those few changes on the way LCOM4 was computed, there were way too many false positives, and nobody was actually using this LCOM4 metric anyway.

Now, at least, the metric is becoming useful as it is able to pinpoint exactly classes where real problems occur (and so now we can indeed think of a rule that looks for classes whose LCOM4 > 1).

If we make any change to the way this metric is computed, it will be to improve it, and not to get back to the original situation where this metric was absolutely useless.

So now, if we want to revert this ticket, the challenge is to not introduce all those false-positives back. One would have to be able to clearly specify what those cases are.

If an instance method does not access any field nor any other instance method, then it should probably be static.

but I do not think that LCOM4 is used to measure individual methods, but rather groups of unrelated methods/fields within a class

It's LCOM4, not LCOGM4

I can imagine only two cases, in which implementing an unrelated single method makes sense

1. You have a utility class, with lots of static methods (java.lang.Math)
2. You derived from a superclass and have to fulfill the superclass' contract. Means you probably have to implement some abstract methods, that are used in the superclass
   • => Take abstract methods into account when calculating the LCOM4 metric. When I'm reading the LCOM4Visitor code correctly, it currently ignores abstract classes. Why would I define an abstract class, when I don't call it. That's a design flaw and should increase the LCOM4
   • => Ignore methods that implement an abstract method from a superclass, because the call is probably made from there, not from within the current class.

All other false-positives can be reviewed and flagged with SONAR-2686. Do you see more reasons to have an isolated method?

Forgot to add:
I agree with you, changes to the way LCOM4 is computed should improve the metric and make it more usable. As it stands right now I feed that it is simplified to unusefulness. So let's try to specify what those false-positive cases are.

Indeed that does make sense.

What about static methods? Should they be ignored in the computation of LCOM4?

Hi,

I think they should ... and they already are I will create a new issue, link it to this one, so that we can start gathering typical "false-positive" patterns.

Oh, I cannot link issues. The new issue is SONAR-3369.

Thanks, link created. Let's continue the discussion there.