76

In a big project I work for, I am considering recommending other programmers to always seal their classes if they haven't considered how their classes should be subclassed. Often times, less-experienced programmers never consider this.

I find it odd that in Java and C# classes are non-sealed / non-final by default. I think making classes sealed greatly improves readability of the code.

Notice that this is in-house code that we can always change should the rare case occur that we need to subclass.

What are your experiences? I meet quite some resistance to this idea. Are people that lazy they could not be bothered to type sealed?

10
  • 28
    Subclassing is rare?!
    – BoltClock
    Jun 17, 2011 at 17:26
  • 13
    How does sealing a class improve it's readability?
    – razlebe
    Jun 17, 2011 at 17:27
  • 4
    Maybe exaggerating a little, but I must have used the sealed keyword on a class I have written like a couple of times in my career. So, no, don't recommend something like this or Anders Hejlsberg will be mad at you. Jun 17, 2011 at 17:27
  • 7
    This is a controversial topic. Personally I wish C# classes were sealed by default too, but there are plenty of smart people on both sides...
    – Jon Skeet
    Jun 17, 2011 at 17:31
  • 9
    If 99% of your classes do not involve inheritance then it would seem the "don't subclass" convention is already entrenched in your org, and you don't need to enforce it. Jun 17, 2011 at 17:36

13 Answers 13

76

Okay, as so many other people have weighed in...

Yes, I think it's entirely reasonable to recommend that classes are sealed by default.

This goes along with the recommendation from Josh Bloch in his excellent book Effective Java, 2nd edition:

Design for inheritance, or prohibit it.

Designing for inheritance is hard, and can make your implementation less flexible, especially if you have virtual methods, one of which calls the other. Maybe they're overloads, maybe they're not. The fact that one calls the other must be documented otherwise you can't override either method safely - you don't know when it'll be called, or whether you're safe to call the other method without risking a stack overflow.

Now if you later want to change which method calls which in a later version, you can't - you'll potentially break subclasses. So in the name of "flexibility" you've actually made the implementation less flexible, and had to document your implementation details more closely. That doesn't sound like a great idea to me.

Next up is immutability - I like immutable types. I find them easier to reason about than mutable types. It's one reason why the Joda Time API is nicer than using Date and Calendar in Java. But an unsealed class can never be known to be immutable. If I accept a parameter of type Foo, I may be able to rely on the properties declared in Foo not to be changed over time, but I can't rely on the object itself not being modified - there could be a mutable property in the subclass. Heaven help me if that property is also used by an override of some virtual method. Wave goodbye to many of the benefits of immutability. (Ironically, Joda Time has very large inheritance hierarchies - often with things saying "subclasses should be immutable. The large inheritance hierarchy of Chronology made it hard to understand when porting to C#.)

Finally, there's the aspect of overuse of inheritance. Personally I favour composition over inheritance where feasible. I love polymorphism for interfaces, and occasionally I use inheritance of implementation - but it's rarely a great fit in my experience. Making classes sealed avoids them being inappropriately derived from where composition would be a better fit.

EDIT: I'd also like to point readers at Eric Lippert's blog post from 2004 on why so many of the framework classes are sealed. There are plenty of places where I wish .NET provided an interface we could work to for testability, but that's a slightly different request...

7
  • 1
    @Paul: It would certainly be nice if C# and Java encouraged composition more... but you can still work with them in a compositional way reasonably easily. And while yes, there can certainly be drawbacks in moving from sealed to unsealed, at least it's feasible - whereas once the genie is out of the bottle, you really can't make an unsealed class sealed in a source compatible way, unless you can guarantee that no-one has derived from the class.
    – Jon Skeet
    Jun 17, 2011 at 18:04
  • 9
    Bloch's advices are generally not applicable to most developers. Most people don't release public APIs to millions of users that last decades. Like OP, they work in-house, the economics of API evolution is completely different. Backward incompatible changes are not only possible and allowed, they are daily activities. Jun 17, 2011 at 18:13
  • 5
    @irreputable: Classes don't have to be public outside a company to be widely used by thousands of people unknown to the original developer, of course. Plenty of developers work in large institutions with large internal systems. I think it's reasonable to try to design an API well, but not to try to design it with every possible subclassing scenario in mind.
    – Jon Skeet
    Jun 17, 2011 at 18:15
  • 1
    @JonSkeet: interesting example. I would contend that the feasibility of moving from sealed to unsealed is the precise problem; users have coded against the expectation of "sealedness"; when that expectation is broken by unsealing the code, the implications will be silent and potentially very dangerous. Making an unsealed class sealed will have IMMEDIATE implications that will, if necessary, break a build. It's the difference of allowing silent failures to creep in through changing expectations, or causing an immediate failure which requires direct attention to fix. I prefer the second. Jun 17, 2011 at 18:19
  • 2
    @Paul: It's definitely something which shouldn't be taken lightly. But at least it's possible. I agree in general, that causing immediate failures is good if there has to be breakage.
    – Jon Skeet
    Jun 17, 2011 at 18:24
19

It is my opinion that architectural design decisions are made to communicate to other developers (including future maintenance developers) something important.

Sealing classes communicates that the implementation should not be overridden. It communicates that the class should not be impersonated. There are good reasons to seal.

If you take the unusual approach of sealing everything (and this is unusual), then your design decisions now communicate things that are really not important - like that the class wasn't intended to be inherited by the original/authoring developer.

But then how would you communicate to other developers that the class should not be inherited because of something? You really can't. You are stuck.

Also, sealing a class doesn't improve readability. I just don't see that. If inheritance is a problem in OOP development, then we have a much larger problem.

10
  • 17
    If a class is sealed, then code using it is more readable because you don't have to consider any variance in the semantics. You know how it will behave more simply.
    – Jon Skeet
    Jun 17, 2011 at 17:31
  • 4
    @JonSkeet: I'd contend that that readability improvement really comes from the implied assurance that the semantics won't change; in the situation posed by the OP, however, the default usage of sealed completely defeats that implied assurance, therefore defeating the readability benefit. Jun 17, 2011 at 17:34
  • 7
    We're going to have to agree to disagree on that one, @JonSkeet. A sealed class could have a long lineage of inheritance itself. Complexity is not contained by sealing it. In fact, sealing it doesn't do anything for readability. It's like you are refering to Sealed as "Locked" or something. Sealing just prevents inheritance. Jun 17, 2011 at 17:34
  • 5
    @Paul: Then we disagree :) I know I'm a lot more comfortable working with sealed classes. Then again, I'm also more comfortable working with immutable classes - and an unsealed class can't guarantee immutability, either...
    – Jon Skeet
    Jun 17, 2011 at 17:35
  • 3
    @JonSkeet: not to get too far into the discussion here in comments, but I find that exhaustive unit tests give me that assurance by explicitly defining the contract expected by the code. Jun 17, 2011 at 17:40
11

I'd like to think that I'm a reasonably-experienced programmer and, if I've learned nothing else, it's that I am remarkably bad at predicting the future.

Typing sealed is not hard, I just don't want to irritate a developer down the road (who could be me!) who discovers that a problem could be easily solved with a little inheritance.

I also have no idea how sealing a class makes it more readable. Are you trying to force people to prefer composition to inheritance?

7
  • 15
    You're right, we can't predict the future - so why not choose the least restrictive option, in terms of future change? Once you've made a class unsealed and someone has derived from it, you can't change that decision. If you want to unseal a class later, that can be done without breaking anything. Don't invite subclassing until you know you need it - at which point you'll know why you need it, so you can design for that aspect of inheritance.
    – Jon Skeet
    Jun 17, 2011 at 17:36
  • 4
    I dont necessarily agree. You have to design a class with subclassing in mind if you really want to add some value to inheritance. What is the point of inheriting a class if you have the same access to its internal state as you would have by simply using a 'has a' relationship. The former entails the use of a protected state and set of protected "tools" which means that you consciosuly have to design the class with subclassing in mind.
    – InBetween
    Jun 17, 2011 at 17:38
  • 1
    @JonSkeet: here's the thing; if I don't want to invite subclassing, I won't write code in an object oriented language that has subclassing and inheritance as a default. In effect, the default behavior of C# (and Java) to not have their classes sealed indicates that those tools are designed with an expectation of subclassing; if you have a desire for the default behaviour to be significantly different, I'd suggest that perhaps you're looking for a different tool. (And yes, I'm aware of whom I'm speaking to, here... :) Jun 17, 2011 at 17:48
  • 3
    @Jon Skeet: if someone's inherited from my class, I have to hope that they had a good reason to do so. Like I said in my cop-out answer, I'm bad at seeing into the future and I'm probably not that bright to boot. I have no problem imagining that someone could figure out a way to solve a problem by inheriting from a class. As far as sealing it until that point, I don't disagree with you, it's just that the point isn't persuasive enough to change my coding habits. You read it as inviting subclassing, I read it as allowing it.
    – 48klocs
    Jun 17, 2011 at 17:49
  • 1
    @48klocs: Consider three developers, who don't know each other. One writes a class, another subclasses it, a third uses it. It's a lot of work for the first developer in order to help the second and third developers to end up with code which can interoperate well...
    – Jon Skeet
    Jun 17, 2011 at 17:51
7

© Jeffrey Richter

There are three reasons why a sealed class is better than an unsealed class:

  • Versioning: When a class is originally sealed, it can change to unsealed in the future without breaking compatibility. However, once a class is unsealed, you can never change it to sealed in the future as this would break all derived classes. In addition, if the unsealed class defines any unsealed virtual methods, ordering of the virtual method calls must be maintained with new versions or there is the potential of breaking derived types in the future.
  • Performance: As discussed in the previous section, calling a virtual method doesn’t perform as well as calling a nonvirtual method because the CLR must look up the type of the object at runtime in order to determine which type defines the method to call. However, if the JIT compiler sees a call to a virtual method using a sealed type, the JIT compiler can produce more efficient code by calling the method nonvirtually. It can do this because it knows there can’t possibly be a derived class if the class is sealed.
  • Security: and predictability A class must protect its own state and not allow itself to ever become corrupted. When a class is unsealed, a derived class can access and manipulate the base class’s state if any data fields or methods that internally manipulate fields are accessible and not private. In addition, a virtual method can be overridden by a derived class, and the derived class can decide whether to call the base class’s implementation. By making a method, property, or event virtual, the base class is giving up some control over its behavior and its state. Unless carefully thought out, this can cause the object to behave unpredictably, and it opens up potential security holes.
1
  • Can you explain this statement: “In addition, if the unsealed class defines any unsealed virtual methods, ordering of the virtual method calls must be maintained with new versions or there is the potential of breaking derived types in the future.” Which method calls is he talking about?
    – user1035721
    Nov 8, 2011 at 13:54
5

There shouldn't be anything wrong in inheriting from a class.

You should seal a class only when theres a good reason why it should never be inherited.

Besides, if you seal them all, it will only decrease maintainability. Every time someone will want to inherit from one of your classes, he will see it is sealed, then he'll either remove the seal (mess with code he shouldn't have to mess with) or worse: create a poor implementation of your class for himself.

Then you'll have 2 implementations of the same thing, one probably worse than the other, and 2 pieces of code to maintain.

Better just keep it unsealed. No harm in it being unsealed.

5

Frankly I think that classes not being sealed by default in c# is kind of weird and out of place with how the rest of the defaults work in the language.

By default, classes are internal. By default fields are private. By default members are private.

There seems to be a trend that points to least plausible access by default. It would stand to reason that a unsealed keyword should exits in c# instead of a sealed.

Personally I'd rather classes were sealed by default. In most ocassions when someone writes a class, he is not designing it with subclassing in mind and all the complexities that come along with it. Designing for future subclassing should be a conscious act and therefore I'd rather you explicitly have to state it.

1
  • It would have been intersting if C# required abstract in order to inherit a class (or some other keyword to indicate it is not sealed). But that's not how it turned out. And I certainly rely on inheriting a class on a whim these days. It could have been. I'm probably glad it didn't turn out like that. Jun 17, 2011 at 17:36
3

"...consider[ing] how their classes should be sub classed..." shouldn't matter.

At least a half dozen times over the past few years I've found myself cursing some open source team or another for a sloppy mix of protected and private, making it impossible to simply extend a class without copying the source of the entire parent class. (In most cases, overriding a particular method required access to private members.)

One example was a JSTL tag that almost did what I wanted. I need to override one small thing. Nope, sorry, I had to completely copy the source of the parent.

2

I only seal classes if I am working on a reusable component that I intend to distribute, and I don't want the end user to inherit from it, or as a system architect if I know I don't want another developer on the team to inherit from it. However there is usually some reason for it.

Just because a class isn't being inherited from, I don't think it should automatically be marked sealed. Also, it annoys me to no end when I want to do something tricky in .NET, but then realize MS marks tons of their classes sealed.

2

This is a very opinionated question that's likely to garner some very opinionated answers ;-)

That said, in my opinion, I strongly prefer NOT making my classes sealed/final, particularly at the beginning. Doing this makes it very difficult to infer the intended extensibility points, and it's nearly impossible to get them right at the beginning. IMHO, overuse of encapsulation is worse than overuse of polymorphism.

2

Your house, your rule.

You can also have the complementary rule instead: a class that can be subclassed must be annotated; nobody should subclass a class that's not annotated so. This rule is not harder to follow than your rule.

1

The main purpose of a sealed class to take away the inheritance feature from the user so they cannot derive a class from a sealed class.Are you sure you want to do that. Or do you want to start having all classes as sealed and then when you need to make it inheritable you will change it .. Well that might be ok when every thing is in house and in one team but incase other teams in future use your dlls it will be not possible to recompile whole source code everytime a class needs to be unsealed .... I wont recommend this but thats just my opinion

1

I don't like that way to think. Java and c# are made to be OOP languages. These languages are designed in a way where a class can have a parent or a child. That's it.

Some people say that we should always start from the most restricting modifier (private, protected...) and set your member to public only when you use it externally. These people are ,to me, lazy and don't want to think about a good design at the beginning of the project.

My answer is: Design your apps in a good way now. Set your class to seal when it needs to be sealed and private when it needs to be private. Don't make them sealed by default.

1

I find that sealed / final classes are actually pretty rare, in my experience; I would certainly not recommend suggesting all classes be sealed / final by default. That specification makes a certain statement about the state of the code (i.e., that it's complete) that is not necessarily always true during development time.

I'll also add that leaving a class unsealed requires more design / test effort to make sure that the exposed behaviours are well-defined and tested; heavy unit testing is critical, IMO, to achieve a level of confidence in the behaviour of the class that appears to be desired by the proponents of "sealed". But IMO, that increased level of effort translates directly to a high level of confidence and to higher quality code.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.