0

Why are the container classes in System.Collections.Immutable, ie ImmutableList<T> sealed ?

I would like to inherit them and have to go through an ugly and error prone composition+proxy ..

Just trying to understand the reason here ?

kofifus
  • 17,260
  • 17
  • 99
  • 173
  • Types should be designed for inheritance which is an added cost to their development. Additionally changing from unsealed to sealed is a breaking change so the decision to unseal should not be taken lightly. I'd say the justification is required in the other direction by default. A compelling reason should be provided for making a type unsealed. What is your compelling reason? – Mike Zboray Jul 10 '18 at 22:26
  • 1
    my compelling reason is providing Equals,== that use SequenceEquals and GetHashCode that combine the hashcodes of the elements inside. Extra cost to development ? we're talking about Microsoft ... – kofifus Jul 10 '18 at 22:29
  • 4
    The notion that Microsoft does not have costs is a strange one. Microsoft has *enormous* costs because the code they develop has to be *bulletproof when used by everyone*. The pool of effort available at Microsoft is large, but it is not infinite, and it is already being spent on solving problems that are more important than making types safely extensible that have no by-design purpose for extending them. Microsoft should spend its finite effort on solving real problems. – Eric Lippert Jul 10 '18 at 22:36
  • 1
    I'm not saying they don't have costs, but I'm not going to to take that as an excuse from one of the richest companies in the world. – kofifus Jul 10 '18 at 22:37
  • 8
    Well I worked at Microsoft for 16 years and believe me, we had hard problems to solve in language, compiler and runtime design, we were hiring the best people we could as fast as we could, and there was **nowhere near enough available effort to do one tenth of the things we wanted to do**. Every moment that Microsoft spends making an unnecessary, dangerous feature because someone *might* want it in the future is a moment not spent solving real user problems. – Eric Lippert Jul 10 '18 at 22:39
  • 1
    sure, but all they had to do is not have the sealed word there and let developers make their own decisions. It's a new world of open not closed and I generally admire Microsoft for embracing that. – kofifus Jul 10 '18 at 22:40
  • 1
    As I often told people who, like you, complained about some feature narrow-focused on their particular scenario not being implemented: the deal you want is Microsoft gives you a free pony. But if we give everyone a free pony, then no one gets the unicorn we could be working on. – Eric Lippert Jul 10 '18 at 22:41
  • 4
    Sure, let's suppose the type is unsealed. That's a feature. It's a feature that is unspecified, untested, and has had no security review. It's a feature that cannot be undone, because sealing is a breaking change, so it will have to be *forever*, even if it is broken. It's a feature that we don't know if any customer needs, it's a feature we don't know if it can be abused. It's a bad feature. It should be cut. – Eric Lippert Jul 10 '18 at 22:42
  • 1
    You're talking as if the other collections are sealed which they are not. I actually think you are wrong here and that the reasoning was different and specific to immutable types – kofifus Jul 10 '18 at 22:44
  • 2
    Moreover: why subclassing? **Why is subclassing the most important kind of extensibility**? People always ask me "why is this type sealed?" as though that question makes any more sense than "why are these fields private?" or "why aren't all the methods actually writable fields of delegate type, so that I can replace them with my own methods at runtime without subclassing?" There are a million kinds of extensibility; what's so great about subclassing? – Eric Lippert Jul 10 '18 at 22:44
  • 3
    That `List` was unsealed was a mistake, a mistake that was not going to be repeated for the immutable types. See https://stackoverflow.com/questions/21692193/why-not-inherit-from-listt/21694054#21694054 – Eric Lippert Jul 10 '18 at 22:45
  • 1
    Subclassing is _the_ classic mechanism of extension, it is extremely convenient. I personally think private methods are wrong design as well, everything should be unsealed and protected to allow maximum flexibility. Instead of being paranoid about misuse think about all the amazingly useful containers that can be written if the classes were not sealed – kofifus Jul 10 '18 at 22:49
  • 3
    Again, you're thinking about this from the perspective of the *developer*. That is a natural perspective to take as a developer but it is the wrong one. **Microsoft thinks about the perspective of the customer**. Remember, Microsoft makes boring software that makes the world work, and their software is *constantly under attack by bad actors*. Anything you can do to reduce points at which a system can be attacked is goodness; anything you can do to make software more reliable is goodness. Seal all your types! – Eric Lippert Jul 10 '18 at 22:51
  • 1
    This is not paranoia. As the saying goes, *if people actually are out to get you, it's not paranoia*. – Eric Lippert Jul 10 '18 at 22:52
  • 2
    Basically, C# got all the defaults right except one. Classes are internal by default, members are private by default, instance methods are nonvirtual by default. The default should have been sealed, but for reasons lost to the mists of time, it was not made the default. – Eric Lippert Jul 10 '18 at 22:54
  • 1
    As said below composition+proxy can be used to do this anyway, IMO all Microsoft achieved with sealed is to make extending more error prone – kofifus Jul 10 '18 at 22:54
  • 1
    I'm saying they made my life much harder without reason, first by not giving a clear way for value semantics on containers and second by then sealing the class. – kofifus Jul 10 '18 at 23:25
  • 5
    `without reason` @EricLippert has explained the reasons (and I would agree with his arguments, but that is beside the point). The reasons may not be compelling **to you** - but it isn't fair to characterise that as 'without reason'. _Your original question stated `Just trying to understand the reason here ?` and that reasoning has been provided by a (very knowledgable!) Microsoft employee. You should mark it as the answer and move on with your life._ – mjwills Jul 10 '18 at 23:34
  • As a side issue, you may wish to raise another question with the actual underlying problem you are trying to solve - in case there may be another way to attack that problem that doesn't need inheritance and / or immutable collections. – mjwills Jul 10 '18 at 23:38
  • 2
    https://github.com/dotnet/corefx/tree/master/src/System.Collections.Immutable/src/System/Collections/Immutable - go copy the source and make your own unsealed version. Microsoft has provided you with a way to do exactly what you want. – MineR Jul 11 '18 at 00:16

1 Answers1

12

All types should be sealed unless they are specifically and carefully designed for extension. Designing for extension is difficult and expensive and easy to do wrong.

Moreover: there are security and correctness implications when you use a type that allows extension. By making the type sealed, the authors of the type are telling the consumers of that type "if you receive an instance of this type, you can rely on the fact that you are actually getting the type that was written by Microsoft, tested by Microsoft, and had the source code published by Microsoft". You can write tests and have confidence that the runtime behaviour will match the test time behaviour because no one else is capable of making their own crazy extension that has a bug.

The question is backwards. You should never ask for a reason for a type to be sealed; sealed should have been the default in the language. Rather, we need a reason to unseal a type: because it was designed for extension, because it was implemented by professionals who carefully understood all the implications of extension, and because consumers of the type were willing to take on the risks of not knowing what the code they're calling actually does.

Eric Lippert
  • 647,829
  • 179
  • 1,238
  • 2,067
  • 3
    Thanks Eric, I personally disagree here, inheritance is a mechanism for extending existing code, if I choose to misuse it in my code I will suffer the consequences in my project, to disallow it by default is playing nanny – kofifus Jul 10 '18 at 22:35
  • Also note that I still can do it (composition+proxy) it just makes it even more error-prone – kofifus Jul 10 '18 at 22:39
  • Thanks for this. I went off googling your writings because I was sure you'd said something about this and lo and behold you provided an answer. You had mentioned in a previous comment that the specific ask here (value equality on generic collections) was a bad idea. Could you expand on that? – Mike Zboray Jul 10 '18 at 22:46
  • @mikez: Upon reflection I deleted the comment. If the assumption is that for an immutable list of `T`, that `T` is well-behaved with respect to value equality, then it *does* make sense for an immutable list to also have value equality. Where you run into trouble is when the generic type is mutable, or where the equality relation is not well behaved. – Eric Lippert Jul 10 '18 at 22:48
  • 1
    @kofifus: You are not following my point. The function of sealing types not intended for extension is not to protect *you* from *yourself* making bad decisions, like a nanny; it is to protect *consumers of the type* from *malicious actors*. You are looking at this from the point of view that types exist for you to extend them to meet your needs, but **most people do not view types that way**. Types exist to *fulfill their function safely and effectively*. When you write a method that uses a sealed type, you *know* what that method does; that's not true of unsealed types. – Eric Lippert Jul 11 '18 at 16:19
  • 1
    I understand your point perfectly and reject it wholly. With this logic the entire .Net framework classes should be sealed which will be a disaster and add 0 security. 'malicious' coders will still write toolkits using your sealed http classes to send passwords to their secret cloud accounts will that be Microsoft's fault ? luckily 99% of frameworks code does not agree with you. I guess we will never agree on this but thanks for answering ! – kofifus Jul 11 '18 at 21:46
  • 4
    @kofifus: Yes, absolutely: *all the framework types should have been sealed from the beginning*. They should have been then unsealed selectively *only* in cases where there was a *documented customer need*, and that process should have involved review at the design, test and implementation stages. Fortunately many of the framework designers have realize that their earlier cavalier attitude was a bad idea, and have started sealing new types. – Eric Lippert Jul 11 '18 at 22:06
  • Great answer, thanks for sharing @EricLippert. It is great to understand the reasoning! – mjwills Jul 11 '18 at 22:35
  • 2
    Eric I was further thinking about this, supposed C# had a new keyword (ie nonvirtual) which unlike sealed would mean that you _can_ derive from the class but you _cannot_ upcast your derived instances to to the class marked as nonvirtual (enforced by the compiler), would that satisfy your security concerns and give us the best of both worlds ? (will also be useful for typedefs ..) – kofifus Jul 17 '18 at 01:13