[swift-evolution] [Review] SE-0006 Apply API Guidelines to the Standard Library

Tue Jan 26 10:25:00 CST 2016

I agree with all of the small criticisms mentioned below by Radoslaw except for the renaming of precondition() to require(). I think it is an improvement that it describes an action now, just like assert().

Count me among those who liked the ‘Type’ suffix for protocols though.

—CK

> On Jan 25, 2016, at 7:40 AM, Radosław Pietruszewski via swift-evolution <swift-evolution at swift.org> wrote:
> 
> Hello all,
> 
> Just like with SE-0005 <https://lists.swift.org/pipermail/swift-evolution/Week-of-Mon-20160125/007658.html>, I’m overwhelmingly for this proposal. The Guidelines, as a whole do a great job at making Swift APIs more consistent and clearer, and applying them to the Swift stdlib is an important first step.
> 
> * * *
> 
> Nitpicks, criticisms and suggestions:
> 
> == precondition was renamed to require ==
> 
> This might be confusing to newcomers, as many languages use the word `require` as a keyword for what we'd call `import`. Although a bit more technical, `precondition` is unambiguous and still easily understandable. I feel like `required` does more damage than good.
> 
> == Removed Type from protocol names ==
> 
> Perhaps I’ve missed some discussion about this and I don’t see the context, but I’m not sure this is a positive change.
> 
> I fear this might be confusing in practice, at least in some contexts. For example, there's nothing signifying that "Boolean" or "Integer" are protocols and not actual types. Same with “Sequence”, “OptionSet”, etc. Perhaps it doesn't matter because everyone will naturally go for `Bool`, `Int`, and `Array` anyway. But I can imagine a lot of confusion if someone tried that anyway, or perhaps saw that in the autocompletion, or the standard library browser (with no intention of using the protocol).
> 
> I’m all for removing unnecessary noise and verbosity, but I think I would err on explicitness side here. It seemed like the -able/-Type convention did a good job disambiguating types you can actually instantiate from protocols, with very little “verbosity cost”.
> 
> == sort() => sorted(), sortInPlace() => sort() etc ==
> 
> I’m torn on this.
> 
> Frankly, I find both the “foo/fooInPlace” and “bar/barred” conventions awkward. Both seem weird. “InPlace” isn’t something I recall seeing anywhere else in API naming, and seems a bizarre way of signifying mutability. “-ed” doesn’t work with all words, so you sometimes have to go with “-ing”, or give up and cry. And then you have inconsistency that “-InPlace” doesn’t seem to have. Also, -ed/-ing can sometimes be difficult to write, especially for non-natives because of the “last letter is doubled” rule for some words.
> 
> But my biggest problem with this change is that IMHO we should encourage to use the transforming (non-mutating) variants by default. One way to achieve this as an API designer and slightly push people towards doing what’s considered best practice is to make the preferable variant easier to type. This might be a subtle change, but I think it matters. Before, if you really wanted to mutate something in place, you had to do that extra little bit of work typing “sortInPlace”, whereas what would be preferable most of the time had a simpler, shorter form: “sort” and would appear earlier in autocomplete.
> 
> == -ings in argument names ==
> 
> I’ve noticed these few tweaks in naming:
> 
>> -  mutating func removeAll(keepCapacity keepCapacity: Bool = false)
>> +  mutating func removeAll(keepingCapacity keepingCapacity: Bool = false)
>> 
>>  public func transcode<...>(...
>> -  stopOnError: Bool
>> +  stoppingOnError: Bool
>>  ) -> Bool
>> 
>>  +  public init(allocatingCapacity count: Int)
> 
> I'm against this change. While I'm not fully convinced of the -ed/-ing rule for methods and properties, it does an important job by conveying the non-mutating semantics of a symbol described. In case of argument names, this rationale no longer applies.
> 
> The only reason to write "stoppingOnError" instead of "stopOnError" is to make method invocations sound more like real English sentences. This is the conventional Objective-C thinking the Guidelines largely step back from. In my opinion, this is futile and provides no readability benefits in this context. Method invocations are _not_ sentences. It's not English, it's code. And while making method names blatantly gramatically incorrect doesn't help readability, neither does forcing `-ing` endings to all boolean function arguments.
> 
> The only thing it does is it adds a few extra characters, an extra word ending the reader has to parse and understand. I know that it's a non-goal to make Swift code as terse as possible, and I'm not arguing for that. But the Guidelines seem to agree that adding extra verbosity _without a good reason_ is a bad thing. Because every extra word and symbol in code just adds to the cognitive load of the reader. And when it doesn't serve a purpose, it just decreases the signal-to-noise ratio.
> 
> Plus, as mentioned before, `-ed/-ing` can be tricky to spell for non-natives. This might not be a big deal, but given that this change provides no benefits, it's one more tiny thing you have to be careful not to get wrong when writing Swift.
> 
> And it's unnecessary:
> 
>    removeAll(keepCapacity: true)
>    transcode(foo, bar, stopOnError: true)
> 
> Are just as clear and readable as:
> 
>    removeAll(keepingCapacity: true)
>    transcode(foo, bar, stoppingOnError: true)
> 
> And the former isn't gramatically incorrect, because this isn't a sentence.
> 
> Apologies for nitpicking on this tiniest possible detail. I just care a lot that we don't create a precedent of trying to make everything sound like English unnecessarily and add verbosity bit by bit.
> 
> * * *
> 
>> 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?
> Yes, and yes, with small details still worth reconsidering.
> 
>> How much effort did you put into your review? A glance, a quick reading, or an in-depth study?
> 
> I’ve read the whole proposal, as well as the related proposals, and read the thread for this review.
> 
> Cross-linking to my SE-0005 review: https://lists.swift.org/pipermail/swift-evolution/Week-of-Mon-20160125/007658.html <https://lists.swift.org/pipermail/swift-evolution/Week-of-Mon-20160125/007658.html>
> 
> Thanks,
> — Radek
> 
>> On 22 Jan 2016, at 22:02, Douglas Gregor via swift-evolution <swift-evolution at swift.org <mailto:swift-evolution at swift.org>> wrote:
>> 
>> Hello Swift community,
>> 
>> The review of SE-0006 "Apply API Guidelines to the Standard Library" begins now and runs through January 31, 2016. The proposal is available here:
>> 
>> https://github.com/apple/swift-evolution/blob/master/proposals/0006-apply-api-guidelines-to-the-standard-library.md <https://github.com/apple/swift-evolution/blob/master/proposals/0006-apply-api-guidelines-to-the-standard-library.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. When replying, please try to keep the proposal link at the top of the message:
>> 
>> Proposal link:
>> 
>> https://github.com/apple/swift-evolution/blob/master/proposals/0006-apply-api-guidelines-to-the-standard-library.md <https://github.com/apple/swift-evolution/blob/master/proposals/0006-apply-api-guidelines-to-the-standard-library.md>
>> Reply text
>> 
>> Other replies
>>  <https://github.com/apple/swift-evolution#what-goes-into-a-review-1>What goes into a review?
>> 
>> The goal of the review process is to improve the proposal under review through constructive criticism and, eventually, determine 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,
>> 
>> -Doug Gregor
>> 
>> Review Manager
>> 
>> _______________________________________________
>> swift-evolution mailing list
>> swift-evolution at swift.org <mailto:swift-evolution at swift.org>
>> https://lists.swift.org/mailman/listinfo/swift-evolution
> 
> _______________________________________________
> swift-evolution mailing list
> swift-evolution at swift.org
> https://lists.swift.org/mailman/listinfo/swift-evolution

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