5

Sonar complaining about private method name in a class when we using the same name of parent private method. In code quality what is the disadvantage of defining a private method with the same name of parent private method?

Or do we need to categorize this as false positive

4 Answers4

2

IMO it's because that could get confusing. Consider below, read the comment:

class Child extends Super{
   public void myMethod() {
     System.out.println("in child");
   }
 }

 class Super{
   public static void main(String[] args) {
    Super s = new Child(); 
    s.myMethod(); // At this point you might expect myMethod of child to be called if it'll call the Parent's since it is private.
  }
   private void myMethod() {
     System.out.println("in super");
   }
 }
Aditya Narayan Dixit
  • 2,105
  • 11
  • 23
  • 1
    The question asks when the two methods are private. In your example, if the two methods are private, you should not wonder whether the Child method may be invoked, you know that only `Super.myMethod()` may be invoked in the current context. – davidxxx Jan 12 '19 at 09:48
2

When you have some method in your subclass with the same name with your superclass, at the first glance, the assumption will be an override, causing confusion when it is not.

The docs mention three situations when this can happen:

The parent class method is static and the child class method is not.

The arguments or return types of the child method are in different packages than those of the parent method.

The parent class method is private.

And also the recommendation:

But if the intent is truly for the child class method to be different, then the method should be renamed to prevent confusion.

So if you really want to not override the method from superclass, the recommendation is to change it to avoid confusion.

You can check an example in the RSPEC-2177 - Sonar Rule Documentation,

The decision of renaming the method or marking the ocurrence as false positive depends entirely on how the team organise their codebase, and the code convention used between the developers.

Community
  • 1
  • 1
nortontgueno
  • 2,553
  • 1
  • 24
  • 36
  • 2
    "So if you really want to not override the method from superclass, the recommendation is to change it to avoid confusion." To avoid confusion, the thing to do is annotated `@Override` the overrided methods. – davidxxx Jan 11 '19 at 17:19
  • @davidxxx I entirely agree with you, but I am mentioning the recommendation presented by Sonar in the docs. – nortontgueno Jan 11 '19 at 17:21
  • Also in some cases you can just add the `@Override` annotation, having a private method in the superclass and a public method in the subclass for example, if you add the `@Override` annotation the compiler will complain since the visibility is not available in the subclass. – nortontgueno Jan 11 '19 at 17:24
  • I know that it is a Sonar recommendation. Just I find it wrong. At each time I did a code review, at least 50 % of the problem are not detected by sonar and at least 50 % of Sonar alerts/issue are not real issues, just details. Whatever your conclusion is correct. It is not to Sonar to decide on this point (+1) – davidxxx Jan 12 '19 at 09:53
0

IHMO, this rule is a no sense.
If the naming makes sense for both the parent and the subclass, you will not invent a different name for one of these to make Sonar happy.
It could make code less clear and also make it less homogeneous in your base code. Private methods are visible only inside the current class, so it is enough to make this choice safe.

davidxxx
  • 125,838
  • 23
  • 214
  • 215
0

I understand the purpose for the rule. However, it should not apply to abstract classes with concrete private methods.

We are using the Apache MINA library, which has several concrete private methods in CumulativeProtocolDecoder that are referenced inside their public concrete methods. If the public methods are overridden, we are forced to provide our own implementations of the private methods. It doesn't make sense to name them something else just to avoid being dressed down by Sonar.

Arlo Guthrie
  • 1,152
  • 3
  • 12
  • 28