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

[Jonathan Hull]

> * What is your evaluation of the proposal?
Still -1.  I think this is a big mistake.

That said, since it seems resistance is futile, I have a few changes I would like to recommend:

First, part of the proposal is aimed at methods which need to be public, but should not be subclassed (at least externally).  There is another equally encountered use-case where a method is intended to be subclassed (it may even be abstract), but it shouldn’t be called by anyone but the originating class (or very occasionally a subclass).  Common examples: drawRect: and layoutSubviews:.  If we had a protected access level, that would solve it, but since we aren’t moving that way either, we should have a keyword that is used in place of ‘open' for these cases (maybe “uncallable” or “hidden”).  We should solve these two together since ‘final’, ‘open’, and ‘uncallable’ are all mutually exclusive states.

Second, I would really like to see the methods match the open-ness or final-ity of their enclosing scope by default.  Note: I also feel this way about access levels, which work this way except for public… I believe they should work the same way for public too (while keeping internal as the default level for top-level structures).  If a class is public open, I am typically going to want 85%+ of the methods to also be 'public open’.  Having to annotate each of them adds a lot of visual noise, and is fairly easy to forget to add when refactoring amidst that visual noise.  I already have this problem with ‘public', so I expect it will only be worse for 'public open'.


Finally, can we talk about migration strategy?  I mean, we are about to break all of CocoaPods and most of GitHub.  At the very least, when migrating to swift 3, we should take any classes/methods which are marked public (but not final) and mark them public open.  That is what the authorial intent was in Swift 2.  This is especially true for people who went through and marked some of their classes/methods final (or thought about it and purposefully didn’t).  Otherwise we are asking users of these frameworks to go in and guess the author’s intent by hand annotating everything while they wait for a fix.

This, more than any other change, will keep me from being able to upgrade to Swift 3 in a timely manner (I have to wait for my dependencies to update).  With a migration strategy, at least I can run the updater on their code as a stop-gap until they have updated their frameworks.

As a framework author myself, this is a tricky update.  I have frameworks which work perfectly well in Swift 2, but are now unusable in Swift 3 without these annotations… but the annotations make them unusable in Swift 2.  I know that is part of the “source-breaking changes” bit, but I have been able to work around pretty much everything else until this.  In the cases where there is a name change causing issues (e.g. “Collection” vs “CollectionType”) they could just run the auto-updater and everything worked.

> * Is the problem being addressed significant enough to warrant a change to Swift?
No, I think it is premature optimization.

There is one legitimate issue of needing to subclass internally without allowing public subclassing.  There are workarounds, but they are workarounds.  Still, this could be solved without changing the default.  I believe changing the default in this manner will create many more problems than it solves. 


> * Does this proposal fit well with the feel and direction of Swift?
No, I believe it makes Swift worse overall.  More visual noise everywhere when reading/writing frameworks.  

I especially think this hurts code reading, which is a hobby/habit of mine.  I will just run through GitHub frameworks reading the code to see how different authors solve problems or organize their code.  I already find having ‘public’ everywhere makes everything less clear/parseable and I suspect that having ‘public open’ everywhere will be strictly worse.  I know that is a less common use-case, but it is something that every programmer should do (and frameworks are, in a way, one of the public faces of swift).


> * If you have used other languages or libraries with a similar feature, how do you feel that this proposal compares to those?
We are breaking new ground here… and not in a good way.

> * How much effort did you put into your review? A glance, a quick reading, or an in-depth study?
I read the proposal and most of the (very long) discussion.

Thanks,
Jon


> Hello Swift community,
> 
> The second review of "SE-0117: Default classes to be non-subclassable publicly" begins now and runs through July 22. The proposal is available here:
> 
> 	https://github.com/apple/swift-evolution/blob/master/proposals/0117-non-public-subclassable-by-default.md <https://github.com/apple/swift-evolution/blob/master/proposals/0117-non-public-subclassable-by-default.md>
> 
> Reviews are an important part of the Swift evolution process. All reviews should be sent to the swift-evolution mailing list at
> 
> 	https://lists.swift.org/mailman/listinfo/swift-evolution <https://lists.swift.org/mailman/listinfo/swift-evolution>
> 
> or, if you would like to keep your feedback private, directly to the review manager.
> 
> What goes into a review?
> 
> The goal of the review process is to improve the proposal under review through constructive criticism and contribute to the direction of Swift. When writing your review, here are some questions you might want to answer in your review:
> 
> 	* What is your evaluation of the proposal?
> 	* Is the problem being addressed significant enough to warrant a change to Swift?
> 	* Does this proposal fit well with the feel and direction of Swift?
> 	* If you have used other languages or libraries with a similar feature, how do you feel that this proposal compares to those?
> 	* How much effort did you put into your review? A glance, a quick reading, or an in-depth study?
> 
> More information about the Swift evolution process is available at
> 
> 	https://github.com/apple/swift-evolution/blob/master/process.md <https://github.com/apple/swift-evolution/blob/master/process.md>
> 
> Thank you,
> 
> -Chris Lattner
> Review Manager

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