17

I like my code to be written well; however, I have run into not really an problem, but more of a question about conventions. Say I have this class.

public class Test {

    public void doStuff() {
        System.out.println("stuff");
    }
}

This class is not going to be extended, so should it be final? I know that when you don't want a class to be extended you make it final; however, in this case the class isn't going to be extended. Sorry if that is confusing. I am basically wondering if you should make every class that isn't going to be extended final.

Jake Anderson
  • 181
  • 1
  • 4

2 Answers2

13

One of the rules for checkstyle that I have a love/hate relationship with is in the Class Design section - Design For Extension. I've often found myself turning it off because it is annoying to follow to a 'T' - especially on large legacy projects where it isn't followed (having a bajillion warnings from check style makes it harder to fix any of them).

That said, its a rather simple rule:

The exact rule is that nonprivate, nonstatic methods of classes that can be subclassed must

  • be abstract or
  • be final or
  • have an empty implementation.

If you can subclass a class, you should make it so that it can be extended in a meaningful way.

"Wait," you say, "This is rule is about public non-static methods that either must be abstract, final, or have an empty implementation. What does this have to do with a final class?"

The exact rule is that non private, non static methods of classes that can be subclassed must ...

If you leave a class non-final, you are saying that it can be subclassed in a meaningful way. If it can't, then it shouldn't be able to be subclassed.

By making a class final, you are making it just a little bit harder for your co-worker to tinker with its innards by making a subclass that exposes all the methods that are implementation specific. There are few things as wonderful as tracking down a bug because an immutable you wrote was subclassed to become mutable - trust me on this.

Designing for inheritance is costly. It takes extra thought to make sure that you are doing it right. That someone else won't mess it up. By saying final class you are avoiding all of that and saying "no - this cannot be inherited" and making it that much easier to reason about.

If you are not spending the time to design the class for extensibility, the appropriate thing to do is make sure no one makes the mistake working from the belief that the class is extensible.

Related reading:

  • It is worth noting that this Checkstyle rule interferes with aspect oriented programming: If the class is final, an aop proxy can not extend the class. While one can work around this by additionally declaring a common interface for the proxy and the implementation class, this can get rather cumbersome, and lead to otherwise pointless duplication of method signatures and their documentation. Do you really want to declare an additional interface for every entity just to use your JPA provider's lazy loading feature? – meriton May 18 '15 at 09:51
  • Bear in mind that making a class final and not offering an interface for it makes it very hard to mock, and that can impede testing. – Martin Epsz May 18 '15 at 11:27
  • Does the "rather simple rule" imply that the [`Decorator`](http://en.wikipedia.org/wiki/Decorator_pattern) pattern violates checkstyle's coding standard (since the base decorator method `doSomething` with non-empty implementation is overridden in concrete decorator method `doSomething` which in turn calls the super method)? – mucaho May 18 '15 at 15:35
  • 2
    @mucaho (and others) the checkstyle rule is one of guidance. It is perfectly acceptable to not run it (I often don't) or ignore it because you have a specific need (such as the Decorator, AOP, mocks without an interface) for the class to behave in a certain way. It is a useful rule for the general case of class design (though a bit nit picky for my tastes many times). The rule is there not as something hard and fast, but firm and flexible - to be broken when need arises (with the understanding of *why* it needs to be broken). –  May 18 '15 at 15:46
  • Then it's ok. I mostly mentioned this because I have seen checkstyle incorporated into mandatory coding standards (as in "code is only done if it has no checkstyle warnings") with this check enabled, and supression forbidden ... – meriton May 19 '15 at 08:42
  • @meriton there are defaults to checkstyle that I vehemently disagree with. Most notably "packageAllowed=false" for Visibility Modifier as that makes testing that much harder (much harder in some cases than final classes) by drastically increasing the size of a 'unit'. Or that "TodoComment" is considered a warning for checkstyle (and under that policy would fail the build). Pmd has its section of [controversial rules](http://pmd.sourceforge.net/pmd-4.2.5/rules/controversial.html). The key for any static analysis is to understand the *why* behind the rule rather than following blindly. –  May 19 '15 at 13:25
  • ... If I am writing a library for the team from scratch, I am quite likely to have even rules on that the team's checkstyle and pmd standards don't mandate and write code that is ultra proper. The goal *behind* the rules is to enable another coder to get into the mind of the original coder faster. Rules that enable this are 'good' and rules that make that harder are 'bad'. Not being able to @SupressWarning or whatever the pmd comment is counter productive. [pmd has the right idea](http://pmd.sourceforge.net/pmd-5.0.4/bestpractices.html) - checkstyle just doesn't state that outright. –  May 19 '15 at 13:32
11

Eric Lippert has an excellent blog post on why one would mark classes final. Specifically, he refers to the Microsoft practice of declaring .NET Framework classes sealed, but the principles are the same.

To summarize, writing an extensible class is a whole different ball game than writing a final class.

  • Sealing classes means you don't have to think about the myriad ways that a consumer might break the class by extending it.

  • Writing extensible classes requires a more rigorous design process. You have to design them to be easily, clearly and logically extensible, a design consideration that you don't have to think about at all if you simply seal the class.

  • You lose encapsulation when you allow a class to be inherited.

  • Supporting an extensible class is much more involved. Yes, you have to support the many ways a consumer might extend it. If you upgrade the class later, you have to worry about breaking consumers' extensions.

See Also
Why aren't classes sealed by default?

Glorfindel
  • 3,137
  • 6
  • 25
  • 33
Robert Harvey
  • 198,589
  • 55
  • 464
  • 673
  • Not sure this is the angle I would take. I guess it depends on the intended audience. It would frustrate me to go to use a library, realise I need to extend a class, then find out I can't because the developer has arbitrarily made the class final (in fact, I have a vague memory of this happening once, but can't remember what the context was!) This is one of situations where open source makes things easier (developer can make it final, and I can fork and make it not final for my use case). – Rikki May 18 '15 at 11:29
  • The point, of course, is that the decision is not arbitrary. – Robert Harvey May 18 '15 at 14:50