Rambling on the sealed keyword

If you want to be a better .NET programmer, there is one book to read from A to Z: CLR via C# by Jeffrey Richter. At page 171, Jeffrey wrote an opinion on the sealed keyword:

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

Personally, I completely agree with this opinion. The result of the sealed keyword choice instead of unsealed is
that the vast majority of .NET programmers let their classes unsealed
without even considering making them sealed. Thus, there are high
chances that the code base you are currently working on is full of
unsealed classes that should be sealed.

The language CQL and the tool NDepend can come to the rescue. The following CQL query lists all classes that are unsealed but that don’t have any derived class.

SELECT TYPES WHERE IsClass AND NbChildren ==0 AND !IsSealed

You should consider carefully the list
of classes matched by this query because they should likely be sealed. Of
course, if you are writing a framework you can have some public unsealed classes
that don’t have any derived class in the context of your framework code base.

 

 

 

This entry was posted in Uncategorized. Bookmark the permalink. Follow any comments here with the RSS feed for this post.
  • http://www.facebook.com/andrew.james.hoffman Andrew Hoffman

    I couldn’t disagree more. You should only seal a class if you have a very good reason or the performance gain is significant. As has been said, a sealed class that does 99.9% of what you need? Throw it out and use another class, which is HORRIBLE.

  • concept.hf

    The JIT may not know this, for example loading assemblies during runtime(ex.: lazy loading)  or dynamic proxying may change the environment. So every time the environment changes, the JIT should reevaluate the calls, and recompile the references, which are quite costly.
    With marking the class sealed however the JIT can conclude, that no recompilation will be needed.

  • concept.hf

    Exposing a method or a member comes with a responsibility from the class itself. If the class can’t assure that altering it’s behaviour by inheritance via an exposed member is not breaking it’s own consistency, then it should simply avoid exposing it. There are the visibility modifiers just for that.
    I can imagine “sealed” to be useful in these scenarios:
    - you mark data objects sealed(for example DTOs) for the purpose of blocking any outside interference, especially if these data objects are handled by some reflection magic.
    - during development if your class is not yet ready for subclassing, and you postpone the polishing by weeks or maybe months. (you can call it ignorant or lazy, but immediate business benefits many times take priority over reusability) Especially if it has public methods not ready to be overriden, but totally ready and safe to use.

    So, I think “sealed” is a design tool that we don’t need in an ideal world, but in reality it can come in handy.  To that end I think the C# team made the right decision by making unsealed the default. Encouraging a class to think about it’s responsabilities is a good thing in the OO world.

  • Brent

    This is probably the first time in my life that I have disagreed with Jeffrey Richter, which I dont do lightly.

    Extensions are nifty, in fact, they are wonderful, but there are places they can’t be used. for instance, suppose I want to create an interface named ICoolControl that has 5 or 6 nice functions for working with UI elements in a uniform way without worrying about what kind of element it really is.

    Now, I go ahead and make a CoolTextbox that inherits TextBox and implements ICoolControl. I can make a CoolCheckbox, ….. except I cant make a CoolPasswordBox because PasswordBox is sealed. Sealed is stupid, there is absolutely no benefit to enforcing sealedness of a class.

    Jeff’s first argument is bunk because if you CANT seal classes, then you have no danger of someday deciding you should, so there is no potential breakage you need to avoid. His second argument is bunk because if you look at the byte codes, the calls are the same, so even if optimization was possible, it isn’t happening. furthermore, that is why we have abstract/virtual/overrides keywords. if a property doesn’t have one of these modifiers, you should be able to optimize it whether or not the class itself is sealed. Jeff’s 3rd point is bunk because when security is important, it can be enforced using private instead of protected. You can use .net reflection to “attack” sealed classes just as well as private class members, so it isnt more “secure” anyway its just more annoying.

    As far as whether or not it is “best” to inherit things just to modify their behavior a little bit or not, who cares? If you don’t think that method of design is effective or efficient or maintainable, don’t use it. The potential ability to subclass doesn’t force you to use it, so why should I be forced not to when I have a good reason, even if good reasons are theoretically rare, sometimes subclassing is exactly what you need.

    So, basically, in the WPF framwork, there is no way for me to get a class that iherits from UIElement implements ICoolControl that masks password entry. Pretty crappy design, all because of the fake security that some guy at Microsoft feels he is getting by sealing the PasswordBox class!!!

  • http://www.e5software.com.mx Miguel

    Where in hell a class should be ‘sealed’, I am trying to create a descendent of SQLConnection and SQLCommand in c# and I can’t create beacause the class is sealed, this is crazy!

  • Kenneth Siewers

    @ Derek Williams
    If you took a look at the overloads for the .Save method you would notice that you can provide your own ImageCodecInfo and EncoderParameters, which completely does what you need. Okay, so you can’t inherit from a class that Microsoft uses as a UTILITY class… So what!? Use what they intend you to use instead of complain about stuff you apparently haven’t investigated thoroughly enough.
    By the way, any good .NET developer knows about a little tool called Reflector… You should take a look at it sometime…

  • mule

    You guys need to read up on the visitor pattern :-)

  • Arunabha

    My answer to the ‘Security and Predictability’ point mentioned: Instead of making the whole class sealed why not making only the virtual functions sealed ? Same for protected properties.

    And regarding performance : Does it mean C# virtual mechanism is not as efficient as C++ ?

  • Robin Theilade

    As long as the .NET team does not have a better idea of when to use the sealed keyword I think that it should be removed altogether. I really think that they generally are doing a great job and I really enjoy to code .NET, but I’m also very tired of running into sealed classes (internal and non-virtual as well for that matter).

    Often the best solution to your problem would have been if the class wouldn’t have been sealed, but instead you have to navigate the .NET framework for some hours before you find another solution that is just remotely acceptable (alright sometimes you are in luck and another solution is just around the corner but it ain’t that often). Sometimes the alternative solution might even require the use of reflection and please don’t tell me that reflection is more performance improving than a non-sealed class would have been.

    My vote: remove sealed

  • Stlan

    I believe the query shoud also exclude static classes by adding ...AND !IsStatic

  • Derek Williams

    What the hell. Sometimes I think people forget the basics.

    Lets say I’d like to write a class that can output an image using it’s Save method, which gets an ImageFormat argument. Fine great, I’ll use the .Net ImageFormat class, happiness.

    What’s that? You want XAML and SVG output, fine, easy peasy to write the XML code; but where is the ImageFormat.XAML, or ImageFormat.SVG. What the hell, I can’t inherit from ImageFormat.

    So I spend an a few hours writing my Own ImageFormat along with the unit tests. If Microsoft hadn’t fricking sealed ImageFormat, I would just inherit from it and add XAML and SVG. But noooo, we’re not fucking worthy. Oh, what’s that you say just write some superduper pattern with some funky interface. Yeah, that makes life easier.

    Those MS Noobs should throw away their pattern language books until they get the damn basics right.

    And by the way your stupid ashx. captcha doesn’t work in firefox, not that real developers use anything but IE.

  • http://tgould.blogspot.com Troy Gould

    i think that most people don’t use the sealed keyword, because it seems to imply that this class should never, ever be inherited from. That’s how I understood it because I first learned what a sealed class was from Java’s string class.

    We have to be clear that we are simply sealing the class for performance optimization. If you really do need inheritance then you are free to remove the sealed keyword.

    However, I do wonder if this is premature optimization in most cases.

  • Ian Cooper

    Rather than inheritance, what most mocking frameworks, and indeed designs want, is an interface to implement. Extensibility comes from providing a new implementation of that interface, polymorphism, far more than through inheritance. Sealed simply indicates that you should not re-use my behavior by inheritance, not that nobody else should not re-implement the same interface, or that no one should not implement in terms of the sealed class.

    Depend on abstractions, not on details.

  • Greg

    per:

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

    This should not be an issue … the JIT compiler is perfectly capable of realizing this at runtime either way. It requires a bit of complexity (need to build a dependency map for methods and be smart enough to code pitch them if your assumptions change)

  • http://www.NDepend.com Patrick Smacchia

    To answer briefly to the bashing on the sealed keyword I would say that inheritance is a reuse mechanism that should be used extremely carefully. You don’t use inheritance as you would use composition or abstraction for example.

    The reason is that inheritance needs this identity things to work properly (Is a vs. Has a). It is quite exceptionnal (although it happens) to find a need for sheer identity while designing some code. What I mean by ‘sheer’ identity is that you won’t regret your inheritance choice in the future. Often we focus our inheritance tree on a particular responsability that ends up not being the root of the identity. The result is code smell and a strong need to refactor as soon as you need to really work with the identity.

    > Developers are likely to want to use it in ways you cannot predict …

    I agree…

    > …and sealing a class impedes some usage scenarios.

    I don’t agree. It is our responsability to avoid client developers to try to invent usage scenario by using wrongly our framework. A good framework should come with clear usage and giving an unsealed class is a very strong indication.

    > Nice post. But I think the CQL sample is too strict. What about internal and private classes?

    You can still refine the CQL constraint such as
    WARN IF Count > 0 IN SELECT TYPES WHERE IsClass AND NbChildren ==0 AND !IsSealed AND IsPublic

    > I do not think all of them should be sealed because they are not visible from the outside world.

    I think that the same level of quality should be applied to internal and public stuff.

  • http://lextm.blogspot.com Lex Y. Li

    There is nothing worse than finding a class you need does 99% of the job but you can’t inherit from it and add the remainder because it’s sealed or non-virtual.
    ================
    Now you can use Extension Method on .NET 3.5 to partially solve it. If you are using CodeGear Delphi for .NET compiler, the Class Helper feature is even more powerful than Extension Method.

    Nice post. But I think the CQL sample is too strict. What about internal and private classes? I do not think all of them should be sealed because they are not visible from the outside world.

  • http://damieng.com Damien Guard

    There is nothing worse than finding a class you need does 99% of the job but you can’t inherit from it and add the remainder because it’s sealed or non-virtual.

    As for the arguments:
    Versioning – that’s like saying if you make a wrong turn you can correct it…
    Performance – if the class is sealed then why does it have virtual methods in the first place?
    Security – yes a class should protect it’s own state when being used but it is not up to the class to ensure subclasses break the contract.

    Designing for use can be tricky but designing or predicting for reuse is incredibly difficult. Marking the class sealed is just plain lazy and ignorant.

    [)amien

  • C-J Berg

    Another common view is that you should never seal a class, especially if you’re developing a library, unless there’s a very good reason to do so (e.g. security). Developers are likely to want to use it in ways you cannot predict, and sealing a class impedes some usage scenarios. See for instance:

    http://www.sellsbrothers.com/news/showTopic.aspx?ixTopic=411
    http://weblogs.asp.net/cazzu/archive/2005/08/29/IHateSealed.aspx

  • Jon Wetzel

    Oh, I see the example he used an override in a sealed class. I was thinking that he was talking about virtual SomeMethod() on a sealed class which didn’t make sense. Valid point.

  • Jon Wetzel

    Yeah, I’m not sure if I agree with the performance thing as a benefit. There is no reason to have a virtual method on a sealed class as far as I know, so that scenario would not present itself. As far as having sealed as the default I’m leaning toward they made the right decision for productivity’s sake.

    Java even has there methods as virtual by default, and C# doesn’t for performance reasons. I’ve heard complaints about that even. I like the C# team’s decision on this as well though. Just my opinion. Good blog, and CLR via C# is a great book all in all.

  • http://www.NDepend.com Patrick Smacchia

    I agree but still, you can use the CQL query if you consider your test code as part of your code base.

    Btw, bonne année Yann :o)

  • Yann Schwartz

    I’m torn on this topic. I agree on the rationale for having classes marked sealed as default, and I’ll go as far as saying that inheritance should be generally considered harmful (multiple inheritance is actually better than the crippled single inheritance we’re stuck with in .Net, and anyway mixins and dynamic proxies are the way to go).

    On the other hand I’ve become addicted to mocking and most of the mocking frameworks throw the towel when faced with sealed classes. As far as I know, only TypeMock can mock a sealed class.

    Sinon, bonne année Patrick.