To seal, or not to seal, that is the question
My current job in Weekend Game Studio is to review the codebase of Wave Engine. We are preparing for a public release, and we want to try to make sure the engine API is as good as we are able to. Even if I have just been playing with it for less than two months, I am the one in charge of the review for one reason: in general the more you work in something, the less you see its problems (this applies to many other things, not just coding).
Of course, given that I have used the engine very little, sometimes I give feedback that simply shows my ignorance of the product, but when the engine team has to explain me why they made a certain decision, they also force themselves to think about it, which helps us a lot in the long run.
On Monday, one of the last things I saw in the code, is that the class Entity was sealed. I spent part of the evening at home remembering a topic that is somewhat controversial when writing a library: whether classes in the library should be sealed or not.
I used to be pro-sealing everything, but my views have changed with time. I think I started to change my opinion after talking in one MVP Summit with Michael Cummings about the subject. Michael has been maintaining the engine Axiom for years, and he was very in favor of not sealing classes (if I remember right :) ). I have also hear this complaint from time to time on forums or blogs when using some libraries (for example XNA).
So while I now think it is better not to seal unless you have a very good reason, I am not totally convinced about it. On Tuesday I decided to put a tweet about the subject, and I got a very interesting conversation with Rodrigo Corral, Jorge Serrano, and Enrique Amoedo.
First thing I usually hit when I have a design doubt is Microsoft Design Guidelines for Developing Class Libraries. In the Design for Extensibility section there are two topics about unsealed and sealed classes. They are pretty short to read, and they seem to be very in favor of not sealing.
Consider unsealed classes with no virtual or protected members as a great way to provide inexpensive, yet much appreciated, extensibility to a framework. By default, most classes should not be sealed.
Do not seal classes without having a good reason to do so. Do not assume that because you do not see a scenario in which extending a class would be desirable, that it is appropriate to seal the class.
There are also quite a few interesting posts about the subject around the internet, most with very long arguments about the topic. This is one of them, which even includes a comment about the subject from Eric Lippert.
The biggest argument usually for sealing, is that if something was not designed (and tested) for extensibility, it should be sealed. Allowing inheritance could break the class, or other classes that depend on that class in ways that are hard to predict, and the cost of maintaining and testing something unsealed is much bigger. Sealing makes the live of the library developer easier (classes cost lest effort), and avoids the user shooting himself on the foot by extending something that was not designed to it (or that the user didn’t understand very well before extending).
The argument for not sealing is that library developers can not imagine every possible use their users may give to a given class, so sealing is forbidding scenarios that may be interesting for the users of such a library. If you have a library that does not allow the users to do what they want, you have unhappy users which is a problem.
In my experience, I have hear very few times users complain because how they shoot themselves on the foot by extending something that shouldn’t be extended. I like the idea of sealing as a way of warning a user that “you should inherit from here at your own risk”, but the implementation of sealed is too restrictive. On the other hand, I have hear quite a few times users complain about not been able to inherit from something sealed.
The biggest problem when sealing is when the sealed class appears as a parameter of a method. In that case, there is no way to pass an specialization of that class to the method, so users need to do pretty strange workarounds. Library developers have a way to avoid this problem, and that is that if the class is sealed, you do not pass it as a parameter, and instead you pass an interface and make your sealed class implement that interface. That is somewhat better, but it seems too clunky on my eyes:
- First, you have created an interface just for the sake of creating an interface. It was not because it made sense, it was just because you needed a workaround.
- Second, you have added more weight to your API and library: the interface, and probably at least one public implementation of it.
- Third, interfaces version very badly, as any change on them will mean a breaking change on every implementation out there. Your users upgrade to the last version of your library, and suddenly nothing compiles. Users hate that a lot in my experience, and I have seen the horror of versioning interfaces (in ArcGIS API) and that is even worse.
The only point to which I can agree is that sealing a class makes things easier for me, the library developer. But given that my final objective is making the live of my users easier and not mine (unless the cost is horrible), I prefer going with not sealing most of the time. On the other side, the only thing that keeps me from being 100% sure about that decision is that unsealing a class is not a breaking change, so depending on how fast we were able to service new versions of Wave Engine, it could become a non-issue (and only unseal when someone finds a case where it would be needed).