1

I got some troubles with a Sonar issue.

I have a bunch of child classes, and I declared a concrete method in the parent class. When called, this method should do something in common for every child class, and then just some of the child classes would override it to do some extra steps.

Parent class method:

 protected void doSomething(Object firstParameter, Object secondParameter) {
       //do something with first parameter
    }

Child class method:

 @Override
     protected void doSomething(Object firstParameter, Object secondParameter) {
       super.doSomething(firstParameter, secondParameter)
       //do something with second parameter
     }

The problem is that the concrete method in the parent class doesn't use secondParameter while the child class does. Therefore, I got a Sonar issue:

Remove this unused method parameter "careCase".

Can you please help? Is it due to bad design? I just want all the child classes to do something when this method is triggered, but just a few of them to do another extra thing (which requires the second parameter)

Jose Robles Villares
  • 33
  • 5
  • I just faced the exact same problem described here. It's definitely not a leaky abstraction as I'm designing a framework for subclasses to use the _unused_ parameters. However, upon reading the [rule description](https://rules.sonarsource.com/java/RSPEC-1172) it became evident that documenting the _unused_ parameter with a JavaDoc gets rid of the warning. That's what I did and it's all good now. – adarshr Mar 07 '23 at 10:38

1 Answers1

0

The design question is whether it's appropriate for the parent to have a dependency on secondParameter. All callers of the parent (and its child classes) will have to add that dependency, whether they would otherwise need to know about secondParameter or not. This may indicate a leaky abstraction.

If you decide it's acceptable to expose the secondParameter in this way, then to avoid an unused parameter warning,

  • One option is to introduce a parameter object so that doSomething() requires only one param.

  • Another option is to implement the "extra thing" as a template method.

      abstract class Parent {
      protected void doSomething(Object firstParameter, Object secondParameter) {
          System.out.println(firstParameter);
          doSomethingElse(secondParameter);
      }
      protected abstract void doSomethingElse(Object secondParameter);
  }

  abstract class DoNothingElse extends Parent {
      @Override
      protected void doSomethingElse(Object secondParameter) {
          // Do nothing.
      }
  }

  class ChildThatDoesSomethingElse extends Parent {
      @Override
      protected void doSomethingElse(Object secondParameter) {
          System.out.println(secondParameter);
      }
  }

  class ChildThatDoesNothingElse extends DoNothingElse {}
jaco0646
  • 15,303
  • 7
  • 59
  • 83
  • But using template method, all the child classes would have to implement doSomethingElse. However, just some of the child classes would actually do something else – Jose Robles Villares Nov 12 '22 at 18:32
  • I was thinking to create an abstract class which extends the Parent class and which has an abstract method which is “doSomethingElse”. Then, only the child classes which need to do this extra thing extend it. And when I need to do this extra thing, I would verify if the class is an instance of this new abstract class. What do you think? – Jose Robles Villares Nov 12 '22 at 18:36
  • From an OOP perspective, if you need to type check the objects, then you have the wrong abstraction, which is another way of saying the classes should not have a common parent. Sharing the common logic through composition is an alternative. You may also insert an additional (potentially abstract) class into the hierarchy, similar to my recent edit above. Note there are several [exceptions](https://rules.sonarsource.com/java/RSPEC-1172) to the Sonar rule. You could simply add a proper JavaDoc for the parameter as well. – jaco0646 Nov 14 '22 at 16:10
  • I just faced the exact same problem described here. It's definitely not a leaky abstraction as I'm designing a framework for subclasses to use the _unused_ parameters. Doing what's been suggested in this answer would be a massive overkill in my case. However, upon reading the [rule description](https://rules.sonarsource.com/java/RSPEC-1172) it became evident that simply documenting the _unused_ parameter with a JavaDoc gets rid of the warning. That's what I did and it's all good now. – adarshr Mar 07 '23 at 10:39