5

I have an interface 

public interface LdapConnectionFactory {
    LdapConnectionWrapper getConnectionWrapper() throws LDAPException;
}

and the implementation class LdapConnectionFactoryImpl which implements the getConnectionWrapper() method.

public class LdapConnectionFactoryImpl implements LdapConnectionFactory {
    ...
    public LdapConnectionWrapper getConnectionWrapper() throws LDAPException {
        ...
    }
}

When I ran checkstyle, it flags the getConnectionWrapper() as error - Method 'getConnectionWrapper' is not designed for extension - needs to be abstract, final or empty. Any advice? Thanks.

PassoGiau
  • 587
  • 2
  • 7
  • 18
  • Take a look at http://stackoverflow.com/questions/5780099/checkstyle-method-not-designed-for-extension-error-being-incorrectly-issued. – snrlx Jul 30 '13 at 12:57
  • 1
    Voting to reopen as the linked "duplicate" is NOT the same question and does not have an accepted answer. – Dave Rager Nov 12 '13 at 19:56
  • Agreed.The linked article covers the case where a final method is incorrectly flagged due to a Checkstyle bug, which is not the case here. A correct duplicate of this issue is http://stackoverflow.com/questions/662598/howto-design-for-extension – mrjmh Mar 03 '15 at 17:10

2 Answers2

6

Checkstyle is saying that either the class or method should be marked as final since it is non-empty and therefore not designed for being overridden.

This is one of the rules of Checkstyle that I don't personally agree with and tend to disable.

John B
  • 32,493
  • 6
  • 77
  • 98
5

Checkstyle is throwing this message because you have provided an implementation but have not marked the method as final. You can disable this check via Eclipse Preferences and editing your Checkstyle configuration. The specific rule is Class Design > Design for Extension.

The description for the check is pretty decent, and is as follows:

Checks that classes are designed for extension. More specifically, it enforces a programming style where superclasses provide empty "hooks" that can be implemented by subclasses. The exact rule is that nonprivate, nonstatic methods of classes that can be subclassed must either be

  • abstract or
  • final or
  • have an empty implementation

Rationale: This API design style protects superclasses against beeing broken by subclasses. The downside is that subclasses are limited in their flexibility, in particular they cannot prevent execution of code in the superclass, but that also means that subclasses cannot corrupt the state of the superclass by forgetting to call the super method.

Nizzo
  • 346
  • 2
  • 6
  • Besides using checkstyle in Eclipse, I'm providing the Ant script to build the application from source which has the checkstyle target. I'm not convinced by this rule rationale, especially when applied to interface implementing class. I'll follow [John B](http://stackoverflow.com/users/905762/john-b) disabling that rule. – PassoGiau Jul 30 '13 at 14:50
  • I don't disagree on disabling the check, and I typically do the same myself. The only time I'll use final is when I'm working on something well defined and I KNOW I wont be extending/etc in the future. Not sure I follow on the 'especially when applied to interface implementing class' though. The fact that you're implementing an interface is moot in this case. The decision on using final for getConnectionWrapper() is more around 'do you want to prevent a subclass from overriding this method' in the future. – Nizzo Jul 30 '13 at 15:41
  • 1
    @Nizzo: Note, you can always remove `final` in the future, without breaking backwards compatibility. However, you cannot add it. Thus, I often find it best to use 'final' by default, and only loosen the restriction later, as needed. Maybe you already know this (: – jwd Oct 07 '14 at 17:07
  • 1
    @jwd: I use final as part of the design of that object to indicate intent. By using final I'm telling the developer that this is a black box and I don't want you overriding functionality. If there is a situation where I could see value in allowing someone to override, I'd prefer to leave it open and not include final. The only challenge I see with your 'final by default' is in cases where your code is used by other developers. This could lead to fragmentation if people were modifying their dependency libraries to suit their needs. For your own projects, it's a luxury you can afford. – Nizzo Oct 07 '14 at 21:30