Welcome to Kartones.Net Sign in

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.

And:

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).

Published 11 April 2012 08:15 by Vicente
Filed under: ,

Comments

# re: To seal, or not to seal, that is the question

11 April 2012 08:29 by Kartones

As you said, library/SDK/API development benefits from sealed classes.

I could even tell you a real world example:

In a past job, where I left sealed the base class of a library I developed before going on vacations to "soft-lock" changes to it until I came back.

My Tech lead was a messy one, trying to touch code but without any idea, and he tried to "extend" my library by adding static methods to the base sealed class instead of to the child ones. He didn't knew why the compiler wouldn't let him, so he didn't touched it.

When I came back the code was "safe and working" and I taught him how to do it at the child classes. Sadly with extension methods he could have done it now :P But that's another scenario.

Giving a closed base contract, in the form of some interfaces and base classes for me is ok. But as long as the contracts are well done (i.e. if everything has an interface, have parameters typed with interfaces, not with classes, so that it is 100% extensible).

For normal usage... I only see it valid maybe on "final components" ought to be used as they are (or else request the owner to extend them), for example Log providers (each one would be sealed/final, while the base one woudln't). But even there I would directly leave the class normal.

# To seal, or not to seal, that is the question

11 April 2012 09:20 by Miemblogs

My current job in Weekend Game Studio is to review the codebase of Wave Engine. We are preparing for

# re: To seal, or not to seal, that is the question

11 April 2012 11:35 by Johann

There is something I don't like about the argument for not sealing classes. Stating that you should leave all your design open for extension implies you should also avoid all private members. Use protected instead. In the same line of thinking, make all your methods virtual, just in case someone needs to override them.

The key problem here is that isolation and open-for-extension are not always compatible. If you take the extreme position of always favoring openness for extension, chances are you will end up with crappier code.

It's hard to argue against inheritance-heavy code because it's hard to provide concrete examples of why it's bad.

The fact is, a lot of, if not most of the IT systems out there suck: unstable, slow, unresponsive. End-users have come to see software systems as very sensitive mysterious gods that easily get angry and get uncooperative for no good reason.

The same can be said about AAA video games. Gamers have come to expect that a new game will really get good and playable after a few patches, around the time the first DLC becomes available.

Next time a developer tells you "I've been using a 'pragmatic' approach to programming for decades and I've always been fine with it", you should ask "What do your end-users think about that? Does the IT help-desk agree with that, or have they come to hate anything you write?"

That being said, I'm not a big fan of going out of my way to prevent extensions. I'm not a big fan of inheritance for code reuse myself, but I won't force my views on others in my code by using sealed. The default for classes in C# is unsealed, and that's fine with me.

# re: To seal, or not to seal, that is the question

11 April 2012 11:40 by Johann

Another comment on this specific part:

"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."

I disagree. Extension and abstraction go hand in hand. If you aren't going to use interfaces in places where you want to provide the ability to extend, where do you use them?

You have created an interface for the sake of extendability. Which is 100% correct. It's not a work-around, it's the (only?) right way to do it.

# re: To seal, or not to seal, that is the question

11 April 2012 18:52 by Vicente

@Johann

I do not think that going for unsealing means also that every member should be protected and every method virtual, that's going to the extreme and I agree with you that it can lead to crappy code.

Allowing to extend a class where every member is private and every method is public allows the user to add some extra functionality to the class that may be interesting for his particular scenario, while making sure his new class continues to be able to play nicely with the existing library.

In a class like that (members private, methods public) a user can not break much more extending than what he could break using the class normally. The biggest thing I can think right now is equality, and that's because that breaks the assumption of everything private/public as it comes from Object.

About interfaces, I like them when they represent a very abstract idea (like IEnumerable) or/and when they represent a very concrete functionality that is not the main concern of the class that will implement them (like IDisposable).

Creating an interface because a class was sealed doesn't fit any of those two criteria. What the user wanted was to add some extra functionality to an existing implementation, not provide a totally different implementation, nor the interface represents a minor concern of the class.

In that context, I think the interface is a clunky solution as it would be over-engineering things, and allowing to extend the class is a simpler solution to the problem. I find that extension in this case is a nice middle point between not allowing anything, and allowing too much.

# re: To seal, or not to seal, that is the question

11 April 2012 20:38 by Unle Luiso

Well, interesting question

I believe the concern is whether there are things designed to be extensible or not. If you have a class you're using and someone extends it, your class consuming that base class is now facing a possibly different beast. The child class can throw exceptions you didn't take into account, can block your thread doing some intense calculations, can add side effects...

Then , I think that not everything has to be extensible. And if something isn't neccesarily extensible, why make it unsealed?

And even if you design some class to be inherited, not every member has to be public. That would be insane. Your base class is responsible for keeping itself stable, keeping its state and functionality. In order to do so, it's reasonable that certain things will be extensible... and other things will not

Then if you ask yourself: should I seal or unseal a class? unsealing a sealed class is something you can do later, when you see a new way of extending. sealing a unsealed class is breaking change. So unsealing a sealed class is a irreversible change. And something you shouldn't do with deep thinking

That's what I think

# re: To seal, or not to seal, that is the question

11 April 2012 21:00 by Vicente

@Luiso

I agree that with a class designed for extensibility (protected members, virtual/abstract methods and properties) you need to think things very carefully. As you say, there are many ways the user could screw himself in that case, so it pays a lot to spend time there designing, testing, and documenting.

But if the class does not have those protected or virtual members, then the user can not really screw the library in many ways, or at least not in ways that depend on the sealing/unsealing decision. Or there are cases I am not seeing right now (which could be :) ).

About unsealing something sealed, it is true that we could do that, but I have no idea right now how fast we would be able to do it in the real world. If we could have  nightly builds (something I would like to have, as users love it), then it would be a very easy to do.

# re: To seal, or not to seal, that is the question

11 April 2012 21:24 by Vicente

@Kartones

Nice example (and really messy tech lead there :p).

I could agree with very final and specialized components if needed, but as you say I would prefer leaving things as unsealed.

@General

It is not as the whole engine is going unsealed (it is not even decided yet :p), and at least we are going to seal the Entity class. Given that our idea is making a component based engine (like Unity) where functionality is achieved via composition (an Entity is a container of Components), sealing Entity allow us to enforce that design vision in our users.

It is a hard decision, but it is too basic and too central to the engine to allow the user to do as he wishes in that case.

But this is also a very extreme case, most classes do not have such a design burden over them as Entity.

# re: To seal, or not to seal, that is the question

12 April 2012 13:07 by Johann

@Vicente: I'm uncomfortable with inheritance for code reuse, which is the scenario you describe.

The key reason is that the claim "Adding to a class cannot break existing functionality in that class" isn't always true. You gave an example using equality. Other cases that bit me include serializability.

In general, any property involving universal quantification is at risk of being broken by inheritance: "instance A and B are equal if ALL their fields are equal" (structural equality), "instance A is serializable if ALL its fields are serializable" (implicit serializability).

For this reason, I can't help disapproving of designs that promote inheritance for code reuse as opposed to inheritance and overriding.

In any case, to know when sealed is appropriate, I suggest taking a look at what the F# compiler does.

F# supports a number of types that aren't explicitly present in C#: records, discriminated unions and tuples. When compiled to MSIL, those become sealed classes. F# also supports classes, which I use to encapsulate behaviour. The other types I use to encapsulate data.

If your project follows the same pattern, I think it's a good idea to make data-centered types sealed (e.g. vectors, colors...) and behaviour-centered types unsealed (game, updatable entities, managers of all kinds...).

# re: To seal, or not to seal, that is the question

13 April 2012 05:56 by Vicente

@Johann

Nice example with Serialization, I haven't thought about it. And more common than equality (which is a strange case when talking about mutable classes).

About the F# compiler, I'll try to give it a look, I know very little of F# sadly :(

In the engine, most basic data centered types are structs. Although the decision for them to be structs is unrelated to this argument (performance mostly).

In our behaviour specific classes we have a few different things going on:

a) A group of abstract classes and interfaces that represent a platform where the engine can run in an abstract way (IMusicPlayer, IIOManager,...).

b) A group of classes that implement the contract exposed before on each specific platform.

c) A group of classes that are heavily designed for extensibility (game Components and game Services mostly) that provide the foundation of the logic.

d) Other helper classes that provide some extra functionality that doesn't fit on the three areas mentioned before.

Most of this debate relates to classes in groups b and d which I am reviewing as best as I can. And thankfully, we have very few data centered reference types :)

# re: To seal, or not to seal, that is the question

13 April 2012 16:44 by Johann

@Vicente

For case b), it does not seem that equality and serialization will be used in practice. Unless you know there are other features that depend on universal properties and aren't safe for inheritance, let them be unsealed.

Case d) is the tricky one. Looking at XNA, I would say PacketWriter is an example of such a helper class. They left it unsealed. It's not a very good argument, but if they do it that way, it must be right :)

# re: To seal, or not to seal, that is the question

13 April 2012 18:06 by Vicente

@Johann

Yeah, in group B serialization is not an issue. Equality could be, as we have a few overrides there (so we may end sealing those just in case). But I have to see how they are being used in the engine, I haven't had time to review that yet.

In group D, it's a lot of utilities and in general not equality or serialization are used anywhere, so they should be pretty safe hopefully :)

# re: To seal, or not to seal, that is the question

14 February 2013 14:44 by Den

Are you ready to support that one paying customer who will inherit your Entity, MainManager and SceneLoader to rebuild half of your engine :)?

It's a lot like JavaScript vs C# - weaknesses come from strengths and vice versa.

# re: To seal, or not to seal, that is the question

14 February 2013 20:24 by Vicente

I can't comment on paid support sadly at this moment :(

But we feel pretty confident in the classes we have sealed and which classes we haven't, but of course, when we release we will read feedback about this very carefully and if we need to change our positions, then we will change them :)

# re: To seal, or not to seal, that is the question

15 February 2013 10:29 by Den

OK, sounds like you've found a right balance, thanks.

# re: To seal, or not to seal, that is the question

15 March 2013 12:14 by Uncle Luiso

Old post, but have just read a new Jon Skeet Post (Jon God Skeet, yes): msmvps.com/.../the-open-closed-principle-in-review.aspx

And I think it is very related. Excerpts:

- "Identify points of predicted variation and create a stable interface around them"

- "design for inheritance or prohibit it"

# re: To seal, or not to seal, that is the question

15 March 2013 20:44 by Vicente

Very nice post from Jon, and great quotes (in general the whole article is great).

Thanks for posting it!

Leave a Comment

(required ) 
(required ) 
(optional )
(required ) 

Captcha