0

Below is a very simple code in Java.

The problem is in the class SC as comments shows. The problem can be easily fixed by removing the final modifier in class C, but it will raise a warning in Checkstyle plugin saying Method 'm' is not designed for extension - needs to be abstract, final or empty.

What is the best design practice here that I should follow?

public interface I {
    public void m();
}

public class C implements I {
    @Override
    public final void m() {
        // some code
    }
}

public class SC extends C {
    @Override
    public final void m(){
        // error message saying "Cannot override the final method from C"
    } 
}
Skyline
  • 145
  • 2
  • 11
  • 2
    The best solution is to remove `final` modifier in `C`. Another may be having a method `m2` in `SC` that calls `m` from `C` but looks like a code smell and should be avoided. – Luiggi Mendoza Aug 19 '14 at 05:42
  • By the way, you should provide a better example for `m` in order to understand why checkstyle raises that warning for it if it can be overridden. – Luiggi Mendoza Aug 19 '14 at 05:43
  • @LuiggiMendoza Thanks for the comment. My example resembles the actual code pretty closely. I assumed that Checkstyle raises the warning for all methods with "@Override" annotation, but I'm not sure. – Skyline Aug 19 '14 at 05:46
  • 2
    I would say the problem is not in your design but in your rules on checkstyle. – Luiggi Mendoza Aug 19 '14 at 05:48
  • 2
    The problem may well be in the design - we have no idea what these classes and interfaces are for. If C hasn't really been *designed* for inheritance, there may well be significant problems in extending it. Could you use composition instead? – Jon Skeet Aug 19 '14 at 05:49
  • Removing `final` in both `C` and `SC` should produce correct code. That warning you quote does not sound right. Is it produced by `C.m()` or by `SC.m()`? – SnakE Aug 19 '14 at 05:52
  • @SnakE It seems that the warning appears when m() in C is not implement nor abstract. Probably since that means that it might not have an implementation anywhere. – Pphoenix Aug 19 '14 at 05:53
  • @JonSkeet Sure, I can use composition instead. Just wondering, does this mean that whenever a class implements an interface, it should never be extended? Since once this class is extended, the Checkstyle warning will occur. – Skyline Aug 19 '14 at 05:56
  • 1
    @Skyline: Well *that* part is just Checkstyle... it's not an unreasonable guideline to aim for - and certainly to understand - but I wouldn't say I follow that 100%. – Jon Skeet Aug 19 '14 at 05:58
  • @JonSkeet Thanks for this. My earlier comment is not entirely correct. I should have said: If a class implements an interface, its subclasses should never override the methods in the interface. I guess it makes sense anyway. – Skyline Aug 19 '14 at 06:00

1 Answers1

1

If m() in C is final, you cannot override it. Since class C implements from interface I, it has to implement all I's methods, that is the way an interface contract is designed.
Note that C does not know of SC. Thus, you should remove final in C and implement m() in C. The implementation could even be blank, depending on what kind of task C is supposed to perform. There is no problem overriding an already existing method (m()) from C in SC.

Pphoenix
  • 1,423
  • 1
  • 15
  • 37
  • Thanks for the reply. Does this mean that I should ignore the Checkstyle warning? Maybe I should turn this warning off completely because of false alarms like this? – Skyline Aug 19 '14 at 05:48
  • @Skyline I am not completely sure what the C class should do. If it is supposed to be a parent class supplying child classes with methods the child classes need to implement I would make C abstract and keep the Checkstyle. If it only happens to be that SC extends C (to perform something extra) I would implement the method in C (thus the warning should be gone) and then let SC call super.m(); TLDR: I would reconsider the design and relation between C and SC and keep the Checkstyle :) – Pphoenix Aug 19 '14 at 05:51