Inheritance is one of the pillar of OOP. However, in the real world, most classes are not designed to be properly inheritable.
Properly designing a class to be inheritable is a tricky task. One must anticipate state initialization, state correctness, state mutability and also non-public methods calls from descendants. Anticipating well code re-use is hard and this requires experience. As a consequence there is no chance that a class gets well designed for inheritance by chance.
However, in both C# and VB.NET, a class is by default inheritable. The C# keyword sealed and the VB.NET keyword NotInheritable must be specified explicitly. Since there is no compiler warning most developers don’t bother to seal their classes. Hence most classes are not properly designed for inheritance but are implicitly declared as inheritable.
As if writing high quality code was not complicated enough here we have a language pitfall.
This is why NDepend proposes the default rule Class with no descendant should be sealed if possible. The CQLinq source code of this rule is pretty obvious:
|
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 |
// <Name>Class with no descendant should be sealed if possible</Name> // <Id>ND1203:ClassWithNoDescendantShouldBeSealedIfPossible</Id> warnif count > 0 from t in JustMyCode.Types where t.IsClass && t.NbChildren ==0 && !t.IsSealed && !t.IsStatic && !t.IsPubliclyVisible // You might want to comment this condition // if you are developing an application, // instead of developing a library // with public classes that are intended to be // sub-classed by your clients. orderby t.NbLinesOfCode descending select new { t, t.NbLinesOfCode, Debt = 30.ToSeconds().ToDebt(), Severity = Severity.Medium } |
As explained in the rule’s comments, you won’t get warnings per default for public non-sealed classes with no descendants. Indeed, it might be well that the analyzed code base is a framework and that descendants of such classes are not available at the time of analysis. But if the code base analyzed is an application with no public API it is important to comment this clause and get most public classes declared as sealed.
If you are not convinced that most classes should be sealed, there are also non-obvious reasons. Here is a quote from Jeffrey Richter from his essential book CLR via C#:
When defining a new type, compilers should make the class sealed by default so that the class cannot be used as a base class. Instead, many compilers, including C#, default to unsealed classes and allow the programmer to explicitly mark a class as sealed by using the sealed keyword. Obviously, it is too late now, but I think that today’s compilers have chosen the wrong default and it would be nice if it could change with future compilers. 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. (…)
- Performance: (…) if the JIT compiler sees a call to a virtual method using a sealed types, the JIT compiler can produce more efficient code by calling the method non-virtually.(…)
- 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 the same vein you can also refer to this Eric Lippert 2004 post where he explains why most .NET Framework classes are sealed per default.
I wish the sealed keyword was part of the default C# file template in VisualStudio. Changing the default templates on even a single machine is a headache and changing them on all machines across the team is almost unthinkable. Another thing that would be great is an opt-in “sealed by default” mode, similar how the new C# 8.0 nullable reference types operate: You set a flag in your .csproj file saying that classes should be considered sealed by default, and then it would be like that for the entire project.
I strongly disagree. Technology should be enabling, not limiting, and developers should be treated as responsible, not as incompetent. The bad guys will always be able to do what they want, sealed or not.
Versioning: Obviously that’s the risk of the inheritor. Nothing much changes for implementors, as sealed classes can break compatibility anyway. Both sides should be aware of what they are doing.
Performance: If you are in situations where such performance difference matters, use structs.
Security and predictability: The answer is even in the paragraph, use private members, which are the default anyway. When someone marks a member as protected where sealing makes a difference, it’s an intentional indication that this member is supposed to be accessed by inheritors.
The penalty of doing what you need with sealed classes (e.g. reflection or type providing) is asymmetrically much worse than the penalty for ensuring your unsealed classes behave the way you want.
Never in my development experience has meeting a sealed type been pleasure. I haven’t ever heard anyone go like ‘oh cool, it’s sealed’ but you can often go ‘oh cool, I can focus on my work rather than reimplementing the whole thing’.
I am not saying that everything that can be done should be done, but I think the team made a good balanced decision with unsealed but private members by default.
Interesting. I’ve found over the years that most of my classes that are “derived” from a base class actually need a much more restricted interface to the base class. As a result I don’t inherit that much but make an instance of the “base” class a private instance variable and then use it internally. What I’m concerned about is someone, including myself, later forgetting the actual interface to the class and using one of the base class methods, bypassing the business logic in my class.
I was going to call you an idiot, and then I decided that might be a bit harsh.
And then I reconsidered. You’re an idiot.