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

Tue Jan 26 18:39:00 CST 2016

on Tue Jan 26 2016, Charles Kissinger <swift-evolution at swift.org> wrote:

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

Interestingly, I was the one that insisted on that change, as I felt
“precondition” was too much of a term-of-art and “require” would be more
accessible, but I am now regretting that decision.  This function is not
conceptually an action; like “assert,” it's a declarative statement, and
“precondition” conveyed that aspect much better, IMO.

>
> 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
>
> _______________________________________________
> swift-evolution mailing list
> swift-evolution at swift.org
> https://lists.swift.org/mailman/listinfo/swift-evolution

-- 
-Dave