Antipattern: Final classes

TL;DR: Don’t declare classes final, and don’t subclass concrete types.

You’ve probably heard the advice. Make your classes final unless they’re open for extension! It’s even in the bible of Java clean code, as item 17 of Effective Java states: “The easier [solution] is to declare the class final.” If you’re unlucky, you’ve had code reviews where nit-pickers get their dopamine fix hunting down every last non-final class in sight. On the face of it, it sounds reasonable: if you haven’t thought about how to make your class extensible, and someone comes along and extends it, they’ve pinned your class down. You can’t change how your methods call each other internally, as that may change the behaviour of subclasses, and adding new methods risks breaking subclass contracts.

As a simple example, suppose somebody has extended your class to log all calls. You now realise your class needs extra functionality, and add a new method. The subclass is silently broken, as the new method will not be logged. Next, you realise you can simplify one method by calling another one a few times. Again, the logging subclass stops working as intended. Someone files a bug against your component asking you to stop making such changes. So, to stop people subclassing your types like that, you apply the f-bomb: making your class final. That breaks the logger folks, but to appease them, you make an interface, and hide your final class behind it. They can now apply Effective Java item 16 and use composition rather than inheritance via the delegation pattern. Problem solved, right?

Well, sort of. That only really solves the problem of changing internal method calls; you still can’t add new methods to the interface without breaking the logger. (Though at least now it will break really loudly, in the compiler, or by throwing linkage errors at runtime.) To fix both, your subclassers will need to turn to proxies (assuming they can algorithmically handle new methods at all; logging is a good example of when this is easy). Or you could create a new interface with the extra methods on, have it extend the old one, and be stuck with them both lying around forever (because nobody cares about deprecation warnings). Or you could provide a few simple base classes for the common patterns your interface is likely to need: for instance, for Java’s List interface, there’s AbstractList, for real implementations, and Guava’s ForwardingList, for delegation.

Prohibition

Now let’s go back in time to when we’re first implementing the class. This time, however, suppose our peer reviewer spots the issue and persuades us to declare our class as final. This theoretically gives us visibility: if someone wants to add logging or whatever, they have to come to us, and get us to split out an interface. In reality, though, that’s just not going to happen in any but the most trivial case. Your class lives in a big company-wide library, and it’s a month until the next release. Or it’s a library used by hundreds of components and they’re all three versions behind already. Nobody’s going to upgrade unless there’s a P0 bug, and even then, it’ll be a quarter before they manage it.

Knowing this, our peer reviewer goes further, and gets us to pull out the interface now, either with an ‘I’-prefix, or an ‘Impl’-suffix, depending on tastes. Oh, and proxies are expensive, so we get to write the delegating implementation, too. With tests. Until another reviewer comes along and asks us to change two of the methods, and rather than rewrite six methods and a dozen tests, we take a trip to Hawaii and wait for someone else to land the code. We’ve fallen afoul of a host of antipatterns, like shotgun surgeryrepetition and code bloat, with no evidence the types will even be used.

Okay, so we manage to push back on the interface extraction, leaving us with a single final class. Now, what options do our erstwhile loggers have open?

Firstly, they could go ahead and write that interface themselves, using the adapter pattern to link it to your implementation. (Pine for duck typing!) Sure, that’s one extra class floating around versus if we’d done the work ourselves, but at least we only do it when it’s actually required, right? Yes…but now it’s done once per client. If we add one new method, they all need to write a dozen new methods to use it. That’s a lot of code.

Secondly, our they could still go ahead and use proxies. Yes, Java 5 can’t proxy concrete classes, but Javassist can, private constructors be damned. This may seem like using a hammer to crack a nut, but it’s actually incredibly common as—although you may not know it—this is how frameworks like Mockito work under the hood, and unit testing is by far the most common reason for substituting implementations. There’s just one problem: we declared our type final.

Whoops.

Temperance

It turns out there’s a much simpler rule that would have saved us from entering this loop in the first place: don’t subclass concrete types.

There’s two ways of looking at this rule, and it’s much more famous in the inverse form: make non-leaf classes abstract. You may still have missed this one, as it comes from Herb Sutter’s More Effective C++, but it’s just as true in Java as it is in C++. Truer, in fact, since in Java we can proxy concrete classes without making fragile subclasses. (Well, technically we’re dynamically creating them at runtime, but that neatly bypasses all the issues with static subclassing.) If we follow this rule in our own code, and assume our users will do the same, we can leave our types non-final, and anyone who subclasses us deserves what they get.

Obviously the rule isn’t absolute. You won’t get very far trying to make exception hierarchies this way, for instance—and of course Object is concrete too, for no really good reason. But the, ahem, exceptions should be rare, and need a much stronger justification than this is the easiest way to do this right now. Otherwise you’re opening your code up to subtle bugs in the future, and when they happen, you won’t be able to blame someone else, because their only fault was not declaring their types final to stop you doing it in the first place.

It’s time to stop the madness, to end the blight, to clean up our code. Don’t use the f-word. Don’t do drugs. And don’t subclass concrete classes.

Advertisements

2 comments

  1. Robert · March 1, 2016

    I agree with all the technical content in this article. Unfortunately there’s a profane non-technical benefit of using (and often even enforcing the usage of) the f-word. Final classes are to programmers what ropes are to climbers: The illustrious 0.01% of Senseis may consider them a hassle, restricting their agility and free (as in speech) unfolding of their mastery (https://www.youtube.com/watch?v=leCAy1v1fnI). For the remaining 99.99% they are probably a wise choice. To an organization with programmers of various levels of knowledge and experience, I would suggest to bias towards the safe option in order to avoid accidental, costly mistakes. There are likely a few exceptions to this proposal, for instance Google who exercises a lot of senior oversight and scrutiny when accepting change requests from junior contributors.

    Like

    • Chris · March 3, 2016

      I disagree that ‘final’ is a wise choice. I don’t see that forcing junior coders to commit shotgun surgery, repetition and code bloat is any more sensible than forcing senior coders do it. Nor is it helpful for junior coders to find Mockito is broken. Whatever mechanism you’ve chosen to adopt to encourage adoption of ‘final’ can just as easily be used to discourage subclassing concrete types, whether that be rabid code reviewers (spotting an illegal ‘extends’ is harder than hunting down missing ‘final’ keywords, after all) or an automatic pre-commit hook. ‘Final’ may look wise, but in practice it is always foolish.

      Like

Leave a Reply

Fill in your details below or click an icon to log in:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out / Change )

Twitter picture

You are commenting using your Twitter account. Log Out / Change )

Facebook photo

You are commenting using your Facebook account. Log Out / Change )

Google+ photo

You are commenting using your Google+ account. Log Out / Change )

Connecting to %s