[swift-evolution] [Review] SE-0117: Default classes to be non-subclassable publicly

[Jonathan Hull]

> On Jul 10, 2016, at 6:45 PM, Rod Brown <rodney.brown6 at icloud.com> wrote:
> 
> @Jonathan
> 
> I don’t think that "pre-breaking code" is a good description. You are not breaking anything - you’re just not allowing something that *could* become unsafe. It’s safety first, at the cost of the library user’s flexibility.

It is pre-breaking in that it is the exact same code that doesn’t work in both cases (only in the pre-breaking case, a bunch of other code also doesn’t work).  I know it feels different because it “was never possible” vs our change being the cause, but it is one of those things like me giving you $5 or giving you $10 and later taking back $5.  As humans we are loss averse so we devise strategies to hide the loss from ourselves.


> That said, I actually think you have a good point however that “sealed” should be able to be overridden, either in a patch capacity or an “unsafe” capacity. Should this become final at a later point, you have acknowledged you know this will be unsafe and are willing to take this risk to get the job done. This is opt-in risk.
> 
> Perhaps however this shouldn’t be “sealed” declaratively. Perhaps we just have a keyword for “Open” as an access level, and if you subclass or override things that are not “open” from other modules, you must mark unsafe.
> 
> I think this is a decent compromise: We allow potential to patch, but discourage without acknowledgement of the risk. Allow Final and Open to be declarative.

Pretty much, but it is important IMHO to separate this from access levels.  They need to be orthogonal concepts or it will be confusing (e.g. why are structs “public” but classes “open”?).  

The default in that case would actually be ‘sealed internal(open)’ which would suck to type repeatedly, but you won’t have to because it is the default.  The thing you would type often would be ‘public open’, which is shorter than the new keywords in the proposal (and consistent for both classes and methods instead of learning different words)

I think we basically agree overall.

Thanks,
Jon


> 
> - Rod
> 
> 
> 
>> On 11 Jul 2016, at 11:05 AM, Jonathan Hull <jhull at gbis.com <mailto:jhull at gbis.com>> wrote:
>> 
>> @Rod:  Thank you for actually replying to the content of my post.  Much appreciated.
>> 
>> It is a trolly problem.  You are arguing that pre-breaking everyone's code is better (even if causes way more trouble overall) than taking an action that breaks a few people’s code later (and thus feeling responsible).  There are other options. I grew up watching enough Star Trek that I don’t believe in no win scenarios.
>> 
>> I still think my compromise is the best solution.  3 levels: Open, Sealed, Final.  The difference is that sealed can be overridden with a compiler warning and use of “unsafe” keyword, but final can’t be overridden.  That way the user is acknowledging that they are doing something which isn’t supported in the context which they are doing it… but it doesn’t completely prevent it by default.  Opt-out safety.  (Yes you lose some compiler optimizations in the default case, but that was premature optimization anyway).
>> 
>> As for the interaction with public/internal/etc… that should be explicit.  This proposal confuses and intermingles them.  You should just be able to say ‘public final internal(open)’ or some other syntax which lets you express the same sentiment.  (The above says the class/method is public and final, but internally it is open.)
>> 
>> I am not saying that we shouldn’t solve these issues.  I am saying this proposal:
>> 1) Isn’t easily discoverable.
>> 2) Requires communication between different parties (author and user) which slows iteration cycles to weeks or months (vs the usual minutes)
>> 3) Conflates/mixes the ideas of access level and subclass-ability
>> 4) Makes way too many separate changes in a single proposal: New idea of “sealed”, new default, replaces ‘public’ in some cases but not others, new keywords (which everyone wants to change), etc…
>> 5) Has structural problems which mean that it won’t actually increase thoughtfulness
>> 6) Makes app developers extremely dependent on framework authors (and those author’s schedules)
>> 7) Will require us to patch/fix it later, but that will be difficult due to optimizations/assumptions of finality.
>> 8) Will cause unnecessary pain for both framework authors and users of frameworks
>> 
>> We would be foolish to accept this proposal without planning for the inevitable escape hatch.  We will need it, and if we don’t plan for it, it will break everything when we are forced to fix it in Swift 4/5.  Anything else is idealistic to the point of ignoring real-world use/behavior.
>> 
>> As I said before, this type of thinking: “If we just make things more difficult, it will encourage awareness” is what leads to the DMV, TSA, and Java.  The problem is, the brain doesn’t work that way, and it ultimately just adds pain without being effective.  You can add forcing functions (like optional unwrapping), which are annoying, but effective… but as I also mentioned before this proposal doesn’t do that.  It is structurally different.  It will not do what you think it does.
>> 
>> Thanks,
>> Jon
>> 
>> P.S.  There is also a dangerous difference between helping the programmer catch mistakes (e.g. don’t accidentally subclass the wrong method) and trying to prevent them from coding in a style you disagree with.  I have been seeing far to many proposals of the second variety of late.
>> 
>> 
>>> On Jul 10, 2016, at 2:58 PM, Rod Brown <rodney.brown6 at icloud.com <mailto:rodney.brown6 at icloud.com>> wrote:
>>> 
>>> I personally agree with most of your assessments. It's why I pushed so hard for "allow subclassing my default" in the first discussion of this point.
>>> 
>>> The problem with this is simple: you cannot retroactively "close up" an API. I cannot add final to a class I have previously declared as non-final. I also can seal a class which has previously been open to subclassing.
>>> 
>>> Consider: someone builds against my framework and I do nothing, and they subclass my classes. Then later I come through and mark the classes as "Sealed". What should we do with those classes that are subclassing my classes? Nothing. I can't. I permitted access and now I'm beholden to that access level.
>>> 
>>> On the other hand, opening up access levels gradually has no such issues. Users of my class can't subclass, and then they can. They just have another tool in the bag now.
>>> 
>>> If you want a default, it should be one you can reverse later. Your default should not be the most restrictive.
>>> 
>>> Whilst I agree with most of your points, this core concept seems to trump them to my mind.
>>> 
>>> - Rod
>>> 
>>> On 10 Jul. 2016, at 5:51 am, Jonathan Hull via swift-evolution <swift-evolution at swift.org <mailto:swift-evolution at swift.org>> wrote:
>>> 
>>>> Please stop saying that this proposal will bring more consideration to the design of libraries.  It isn’t true.  I haven’t even seen an argument for why it would be true, it is just taken for granted that it is true.
>>>> 
>>>> As I mentioned in another post, this is structurally very different from things like ‘if-let’ and optionals.  Optionals force the user to consider their decision in the context it is being used (i.e. as you use the optional/value).  This proposal, however, does the opposite.  The effect of your actions appear in the context of a completely different user.  It is like sitting in a room, flipping a light switch wondering “I wonder what this does?”… meanwhile the people downstairs are wondering why their lights keep turning off and on”.  
>>>> 
>>>> You can try to test for this, but by definition library authors can only test for scenarios that they have thought of.  I have often found people surprise me with their use-cases.  Relying on the diligence of other programmers is what leads to things like: "You always need to remember to test for zero before using a pointer".  Literally the opposite of optionals!  It sounds good, but at the end of the day, people are human and they WILL make mistakes.  Best to either catch those mistakes in the context where they happen or to mitigate the effect of it.  This proposal basically forces you to feel the full effect of other people's mistakes (thinking that it will discourage them from making them in the first place).
>>>> 
>>>> Your only real mechanism for feedback is when users of your library complain to you that something that they need isn’t subclass-able.  This is the point where most framework authors will actually learn that this feature/default exists.  Users of a framework will learn of it slightly earlier, when they find they need to subclass something, and it just isn’t possible.
>>>> 
>>>> 
>>>> I would much prefer adding a ‘sealed’ keyword which library authors could use to annotate things which they do not want subclassed outside of the module.  Or preferably, as others have suggested, allow augmentation of ‘final’ with ‘public(final)' or ‘internal(final)’.
>>>> 
>>>> The only case where I would support ‘sealed’ by default is if there are 3 levels: open, sealed, final.  Final would allow 'public(final)' and 'internal(final)’ to allow private subclassing inside the file/module.  Sealed would be the same, except it would allow the user to subclass by explicitly acknowledging the risk using ‘unsafe’:  “unsafe class MySubclass:SealedSuper“ and  “unsafe override func”.  Final would not allow the override.
>>>> 
>>>> That is the case where ‘sealed’ makes sense as a default…
>>>> 
>>>> Thanks,
>>>> Jon
>>>> 
>>>> P.S. The current proposal will only cause massive problems down the line, IMHO.  We will find an escape hatch is needed, but we will have made optimizations based on assumptions of finality which prevent us from easily adding one.
>>>> _______________________________________________
>>>> swift-evolution mailing list
>>>> swift-evolution at swift.org <mailto:swift-evolution at swift.org>
>>>> https://lists.swift.org/mailman/listinfo/swift-evolution <https://lists.swift.org/mailman/listinfo/swift-evolution>
>> 
> 

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.swift.org/pipermail/swift-evolution/attachments/20160710/7920605e/attachment.html>