[Review] SE-0023 API Design Guidelines


(Douglas Gregor) #1

Hello Swift community,

The review of SE-0023"API Design Guidelines" begins now and runs through January 31, 2016. The proposal is available here:

https://github.com/apple/swift-evolution/blob/master/proposals/0023-api-guidelines.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
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/0023-api-guidelines.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
Thank you,

-Doug Gregor

Review Manager


(Joe Groff) #2

When a mutating method is described by a verb, name its non-mutating counterpart according to the “ed/ing” rule, e.g. the non-mutating versions of x.sort() and x.append(y) are x.sorted() and x.appending(y).

This is a nice rule in theory, but English fights it with the full fury of its irregularity. There are a lot of common operations whose past tense shares a spelling with the infinitive—'split', 'cut', 'read', and 'cast' immediately come to mind. How do you handle naming non-mutating versions of these operations? Conjugating other irregular verbs also imposes a barrier on developers whose first language is not English.

-Joe


(Erica Sadun) #3

Current:
Use imperative verb phrases for mutating methods: x.reverse(), x.sort(), x.tweak()
Use noun phrases for non-mutating methods: x.distanceTo(...), idx.successor()
Proposed:
Use verb phrases to declare procedural methods, whether or not they mutate an instance or just produce side effects: x.reverse(), x.sort(), x.tweak(), x.perform(), x.dispatch(), x.send()
Use noun phrases to describe values returned by a functional method: x.distanceTo(y), index.successor() (This admittedly leaves further issues around other functional methods, for example, seq.separatedBySequence(seq) and int.strideTo(other: Self, step:Self.Stride), etc. )
I suggest that mutating methods are just a procedural method (side effect, no return value) vs functional.

-- E

···

On Jan 22, 2016, at 2:02 PM, Douglas Gregor via swift-evolution <swift-evolution@swift.org> wrote:

Hello Swift community,

The review of SE-0023"API Design Guidelines" begins now and runs through January 31, 2016. The proposal is available here:

https://github.com/apple/swift-evolution/blob/master/proposals/0023-api-guidelines.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
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/0023-api-guidelines.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
Thank you,

-Doug Gregor

Review Manager

_______________________________________________
swift-evolution mailing list
swift-evolution@swift.org
https://lists.swift.org/mailman/listinfo/swift-evolution


(David Owens II) #4

Compensate For Weak Type Information as needed to clarify a parameter’s role.

Especially when a parameter type is NSObject, Any, AnyObject, or a fundamental type such Int or String, type information and context at the point of use may not fully convey intent. In this example, the declaration may be clear, but the use site is vague:

func add(observer: NSObject, for keyPath: String)
grid.add(self, for: graphics) // vague

To restore clarity, precede each weakly-typed parameter with a noun describing its role:

func addObserver(_ observer: NSObject, forKeyPath path: String)
grid.addObserver(self, forKeyPath: graphics) // clear

I don’t understand why to compensate for weak type information we put some of that compensation in the name of the function and other parts of it in the [external] name of the parameter.

If we were going to reference functions like this: addObserver:forKeyPath, then I can understand it. But that’s not the plan of record, it’s to do this: addObserver(_:forKeyPath).

Regardless of the default naming scheme, it seems like the rule should be to use external names to clarify that parameters role.

func add(observer observer: NSObject, forKeyPath path: String)
grid.add(observer: self, forKeyPath: graphics)

This promotes a very clear and consistent rule: weak type information should be clarified by the parameter’s external name. There are no exceptions for the first parameter. Otherwise, it seems like there is super fine line between this rule and the next one below.

Additionally, this also alleviates my concerns with the default parameter have _ as the external name by default because this addresses the case when it would be desirable to have that name. Further, the case below handles the case when it’s not.

Omit Needless Words. Every word in a name should convey salient information at the use site.

More words may be needed to clarify intent or disambiguate meaning, but those that are redundant with information the reader already possesses should be omitted. In particular, omit words that merely repeat type information:

public mutating func removeElement(member: Element) -> Element?
allViews.removeElement(cancelButton)

In this case, the word Element adds nothing salient at the call site. This API would be better:

public mutating func remove(member: Element) -> Element?
allViews.remove(cancelButton) // clearer

Occasionally, repeating type information is necessary to avoid ambiguity, but in general it is better to use a word that describes a parameter’s role rather than its type. See the next item for details.

The description here seems to overlap with the “Compensate for Weak Type Information” rule, especially with the clause: “repeating type information”. It may be better to re-work the example to be `removeItem(member: Element)` to make this distinction more clear that it’s not type information being removed.

Also, by clarifying that statement, the above rule change I suggested would be consistent. Type information clarification goes into the external parameter name, functionality clarification goes into the function name. Those are hard-n-fast rules that are straight-forward.

Be Grammatical

When a mutating method is described by a verb, name its non-mutating counterpart according to the “ed/ing” rule, e.g. the non-mutating versions of x.sort() and x.append(y) are x.sorted() and x.appending(y).

Is this guideline suggesting that we should design our APIs to generally have both mutating and non-mutaging counterparts?

As other have pointed out, this is also very hard to do all the time. I think the alternatives are worse. It would be nice if there were a way to annotate all member functions as mutating/non-mutating to really by-pass this ambiguity.

Other than the above, the proposal looks pretty good to me.

-David


(Matthew Johnson) #5

This review seems like a good time to bring up the proposal regarding naming conventions for conversion protocols: https://github.com/apple/swift-evolution/pull/60. Dave had mentioned having the standard library team discuss this as part of the API guidelines: https://lists.swift.org/pipermail/swift-evolution/Week-of-Mon-20151214/002798.html. I’m wondering if the solution for this naming issue should continue as an independent proposal or be included as part of the the larger API Design Guidelines proposal.

-Matthew

···

On Jan 22, 2016, at 3:02 PM, Douglas Gregor <dgregor@apple.com> wrote:

Hello Swift community,

The review of SE-0023"API Design Guidelines" begins now and runs through January 31, 2016. The proposal is available here:

https://github.com/apple/swift-evolution/blob/master/proposals/0023-api-guidelines.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
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/0023-api-guidelines.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
Thank you,

-Doug Gregor

Review Manager

_______________________________________________
swift-evolution-announce mailing list
swift-evolution-announce@swift.org
https://lists.swift.org/mailman/listinfo/swift-evolution-announce


(Trent Nadeau) #6

Under "Follow case conventions", how should acronyms (like "HTML") be
handled: HTMLElement or HtmlElement?

···

On Fri, Jan 22, 2016 at 4:02 PM, Douglas Gregor via swift-evolution < swift-evolution@swift.org> wrote:

Hello Swift community,

The review of SE-0023"API Design Guidelines" begins now and runs through
January 31, 2016. The proposal is available here:

https://github.com/apple/swift-evolution/blob/master/proposals/0023-api-guidelines.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

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/0023-api-guidelines.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

Thank you,

-Doug Gregor

Review Manager

_______________________________________________
swift-evolution mailing list
swift-evolution@swift.org
https://lists.swift.org/mailman/listinfo/swift-evolution

--
Trent Nadeau


(Thomas Visser) #7

Uses of non-mutating methods should read as noun phrases when possible, e.g. x.distanceTo(y), i.successor().

This seems to indicate (to me) that e.g. Optional.map and should be renamed to Optional.mapped? Except that ‘map’ in this case could be considered a ’term of art’. Would it make sense to add to add some kind of guidance on how to weigh the individual guidelines when they are conflicting?

Thomas

···

On 22 Jan 2016, at 22:02, Douglas Gregor via swift-evolution <swift-evolution@swift.org> wrote:

Hello Swift community,

The review of SE-0023"API Design Guidelines" begins now and runs through January 31, 2016. The proposal is available here:

https://github.com/apple/swift-evolution/blob/master/proposals/0023-api-guidelines.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
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/0023-api-guidelines.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
Thank you,

-Doug Gregor

Review Manager

_______________________________________________
swift-evolution mailing list
swift-evolution@swift.org
https://lists.swift.org/mailman/listinfo/swift-evolution


(Kevin Lundberg) #8

*Protocols* that describe what something *is* should read as nouns

(e.g. |Collection|). Protocols that describe a *capability* should be
named using the suffixes |able|, |ible|, or |ing| (e.g. |Equatable|,

ProgressReporting>).

I personally like the idea behind the current convention for protocols
that describe a thing (IntegerType, CollectionType, etc) where there is
a suffix of Type appended to the end, so I give this specific part of
the proposal a -1. The specific wording of the protocol's name is not so
important as the recognition at a glance that this is a protocol vs a
concrete type. I like being able to infer at a glance how I'm expected
to use a specific type reference based on its name alone; otherwise I
may have to refer back to the type definition to refresh my memory of
whether or not it is in fact a protocol or is something else.

This change could also lead to confusion among some developers. For
someone who is new to Swift, would they know they should use Bool over
Boolean if they've seen both types before? Both names look reasonable to
store a boolean value, but the semantics of each type differ
significantly. Someone may try to have a type conform to Bool instead of
Boolean, which would obviously not work, but could cause some
consternation for developers who don't know the difference by heart.
Naming the protocol BooleanType at least calls out that this may not be
conceptually the same as a plain boolean value, which could make a
developer think twice before trying to use that over Bool.

Removing some common prefix from these kinds of protocols could also run
the risk of unintentionally shadowing type names, if someone wanted to
write their own Collection or Error struct or class for instance, or if
a pre-existing concrete type in their code turned out to unexpectedly
shadow a protocol in a new dependency that they want to add. These
situations would not cause any technical hiccups due to module
namespacing, but it could lead to confusion when a developer forgets to
qualify the name and tries to use one type where the other is expected.

In short, appending Type (or something like it) i think is a reasonable
convention to keep around for non-behavioral protocols.

As far as alternatives to 'Type', I personally don't like the suffix
'Protocol' as much (which is suggested as a disambiguation option in the
related standard library review), since 'Type' is shorter, feels nicer
to read, and describes the purpose of the protocol well to me. C#'s
approach of prefixing all interfaces with a capital I would be even more
succinct, but I personally don't think that approach would look nice to
read either. (PCollection, PBoolean? Ick.)


(Jacob Bandes-Storch) #9

Proposal link:
https://github.com/apple/swift-evolution/blob/master/proposals/0023-api-guidelines.md

What is your evaluation of the proposal?

I think there are more things which would make a "guidelines" document even
better for promoting consistent code:

- When to use functions vs. properties for getters. The changes in SE-0006
<https://github.com/apple/swift-evolution/blob/master/proposals/0006-apply-api-guidelines-to-the-standard-library.md>
include
"func min()" and "var underestimatedCount". SE-0005
<https://github.com/apple/swift-evolution/blob/master/proposals/0005-objective-c-name-translation.md>
includes "class func darkGray()". I imagine it should be implied that
properties are more efficient to compute, but this doesn't seem to be
applied consistently.

- Edge cases of naming, such as what to do with acronyms (like URL) when
they appear in function and variable names, especially at the beginning.

- When to use structs/classes/enums; when to use optionals; other such
basic API design topics.

While certainly not everything from the Coding Guidelines for Cocoa
<https://developer.apple.com/library/mac/documentation/Cocoa/Conceptual/CodingGuidelines/CodingGuidelines.html>
(linked from SE-0005) applies to Swift, the Swift guidelines would do well
to match its thoroughness.

    Protocols that describe what something is should read as nouns (e.g.
Collection).
    Protocols that describe a capability should be named using the suffixes
able, ible, or ing (e.g. Equatable, ProgressReporting).

"SetAlgebra" (from SE-0006) doesn't really fit in with this.

Extremely minor:
    text = "The value is: "
    text += String(veryLargeNumber)
    text += " and in hexadecimal, it's"
    text += String(veryLargeNumber, radix: 16)

The second string literal should have a space at the end as well as the
beginning.

Is the problem being addressed significant enough to warrant a change to

Swift?

Having a set of guidelines is important.

Does this proposal fit well with the feel and direction of Swift?

Yes.

How much effort did you put into your review? A glance, a quick reading,

or an in-depth study?

Fairly in-depth.

Jacob

···

On Fri, Jan 22, 2016 at 1:02 PM, Douglas Gregor via swift-evolution < swift-evolution@swift.org> wrote:

Hello Swift community,

The review of SE-0023"API Design Guidelines" begins now and runs through
January 31, 2016. The proposal is available here:

https://github.com/apple/swift-evolution/blob/master/proposals/0023-api-guidelines.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

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/0023-api-guidelines.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

Thank you,

-Doug Gregor

Review Manager

_______________________________________________
swift-evolution mailing list
swift-evolution@swift.org
https://lists.swift.org/mailman/listinfo/swift-evolution


(Dave Abrahams) #10

FYI, a number of minor changes were made to the development version of
the guidelines at
http://apple.github.io/swift-internals/api-design-guidelines/ that may
make it easier for you to evaluate:

* When the page is printed, all sections are always shown expanded.

* A button was added to the top of the page that expands or collapses
  all details sections.

* Presentation tweaks were made to:
  * Help the color-impaired interpret code examples
  * Omit needless graphical clutter.

* A series of copyedits by our editorial department were applied. These
  are mostly "gardening," as @practicalswift would put it; they
  eliminate inconsistencies and bring it "up to code" w.r.t. Apple
  editorial practices.

No substantial changes were made to any of the content, so this version
should be a suitable substitute for
https://swift.org/documentation/api-design-guidelines/ for the purposes
of the review.

Cheers,

···

on Fri Jan 22 2016, Douglas Gregor <swift-evolution@swift.org> wrote:

Hello Swift community,

The review of SE-0023"API Design Guidelines" begins now and runs
through January 31, 2016. The proposal is available here:

https://github.com/apple/swift-evolution/blob/master/proposals/0023-api-guidelines.md

--
-Dave


(Joe Groff) #11

This all looks good to me (aside from the linguistic problems with verb conjugation I've raised in another subthread). However, I think these naming guidelines lead us to reconsider our default argument labeling rules for 'func' declarations again, as David Owens and others have suggested. The stated goal of the current language rule is to guide people into good API design following our conventions, but I don't think it succeeds in serving that purpose. If you follow the guidelines, the argument labels for your secondary arguments generally end up becoming prepositional phrases, which make for poor variable names, and you're naturally guided to giving the argument an explicit descriptive binding name:

func perform(stuff: Stuff, with: Thing) {
  with.apply(stuff) // 'with' is a weird variable name
}

func perform(stuff: Stuff, with thing: Thing) {
  thing.apply(stuff) // 'thing' is better
}

The shorthand thus doesn't save the good API citizen from much work. On the other hand, a developer who's unaware or uninterested in the guidelines and is just trying to C or Java in Swift gets argument labels by default that neither follow the guidelines nor meet their expectation:

func atan2(y: Double, x: Double) -> Double { ... }

atan2(10, 10) // Why doesn't this work?
atan2(10, x: 10) // Nobody wants this

And when staring down potentially dozens or hundreds of compile errors at various mismatched use sites, they're unlikely to reconsider their API naming choice, and will instead do the minimal amount of work to get their code to compile by suppressing the argument label. The language hasn't led this developer to better conventional API design either.

I can think of a couple possible modifications to the language rule that could help reduce the surprise factor, and still lead people to good API design:

- Require all 'func' arguments after the first to explicitly specify both a label and a binding name. Users following the guidelines will usually end up doing this anyway, and users who aren't will get a helpful message instead of unexpected behavior. This also avoids a problem with our current rule, where otherwise identical-looking parameter declarations in a 'func' end up behaving differently based on position. A diagnostic immediately at the declaration site also seems more likely to me to lead the developer to think more about their API naming; diagnosing call sites that don't look the way they want is just going to lead them to reactively suppress the labels to get their code to compile.
- Change the default rule so that all arguments *after an explicitly labeled argument* default to being labeled (instead of all arguments after the first). It's unlikely anyone wants an unlabeled argument positionally after a labeled one, and the rare methods in Cocoa that take more than two arguments do tend to use noun rather than preposition phrases for arguments after the second. Users following the guidelines get nice labeled APIs, and users who aren't get the bare, uncaring anonymous arguments they deserve:

func perform(stuff: Stuff, with thing: Thing, options: StuffOptions) // perform(_:with:options:)
func atan2(y: Double, x: Double) // atan2(_:_:slight_smile:

-Joe

···

On Jan 22, 2016, at 1:02 PM, Douglas Gregor via swift-evolution <swift-evolution@swift.org> wrote:

Hello Swift community,

The review of SE-0023"API Design Guidelines" begins now and runs through January 31, 2016. The proposal is available here:

https://github.com/apple/swift-evolution/blob/master/proposals/0023-api-guidelines.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
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/0023-api-guidelines.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
Thank you,

-Doug Gregor

Review Manager

_______________________________________________
swift-evolution mailing list
swift-evolution@swift.org
https://lists.swift.org/mailman/listinfo/swift-evolution


(Marc Knaup) #12

What is the rationale behind naming enumeration cases in upper camel case?

*Follow case conventions:* names of types, protocols and enum cases are

UpperCamelCase. Everything else is lowerCamelCase.

let a = NSComparisonResult.OrderedSame // refers to a value, but is
upper-case
let b = NSDate.distantFuture // refers to a property/value, but
is lower-case

   - everything related to types (type names, protocol names, generic type
   parameter names) should be upper camel case
   - everything else (function names, property names, variable names, etc.)
   should be lower camel case

This is already the current and the proposed recommendation with
enumeration cases being the only exception.
Enumeration cases are not types.

···

On Fri, Jan 22, 2016 at 10:02 PM, Douglas Gregor via swift-evolution < swift-evolution@swift.org> wrote:

Hello Swift community,

The review of SE-0023"API Design Guidelines" begins now and runs through
January 31, 2016. The proposal is available here:

https://github.com/apple/swift-evolution/blob/master/proposals/0023-api-guidelines.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

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/0023-api-guidelines.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

Thank you,

-Doug Gregor

Review Manager

_______________________________________________
swift-evolution mailing list
swift-evolution@swift.org
https://lists.swift.org/mailman/listinfo/swift-evolution


(Douglas Gregor) #13

When we discussed this, we felt like “map” was a term of art. It’s also the case that, generally speaking, “map” cannot be mutating unless you’re mapping from T -> T.

As for general guidance on how to weigh the guidelines… I’m not sure we could write anything here that would actually help. There is always judgment involved in API design, and “when should one prefer a term of art?” is very much a judgment call.

  - Doug

···

On Jan 22, 2016, at 2:04 PM, Thomas Visser <thomas.visser@gmail.com> wrote:

Uses of non-mutating methods should read as noun phrases when possible, e.g. x.distanceTo(y), i.successor().

This seems to indicate (to me) that e.g. Optional.map and should be renamed to Optional.mapped? Except that ‘map’ in this case could be considered a ’term of art’. Would it make sense to add to add some kind of guidance on how to weigh the individual guidelines when they are conflicting?


(Paul Cantrell) #14

I really like the general spirit of these guidelines. I particularly appreciate their emphasis on clarity at the point of use, and their recognition that both brevity and verbosity can be the enemy of clarity.

Some of the particulars of the guidelines haven’t worked well for me in practice — and I see from this thread that others have hit some of the same problems. The guidelines as they stand feel like they’ve mostly been vetted against a bunch of ADTs (which is probably true — stdlib?), and could use exposure to a broader range of libraries.

Immediately after the guidelines were published, before this review began, I tried applying them to Siesta. The project has 244 public declarations, of which I found 28 that the guidelines seemed to recommend changing. Of those 28, after much reflection and discussion, I ended up changing only 7 to match the guidelines (and also cleaning up several others, but in a non-guideline-compliant way).

In short, in a real-world project, pre-guidelines code agreed with the guidelines 89% of the time, but where it disagreed, the guidelines achieved only a 25% acceptance rate with the curmudgeonly developers.

You can follow that discussion here:

  https://gist.github.com/pcantrell/22a6564ca7d22789315b
  https://github.com/bustoutsolutions/siesta/issues/15

Several places where we rejected the guidelines relate to the raging debate about first argument labels. Here’s a rundown of those particular cases. (Since this message will be a long one, I’ll share in a separate message some notes on the other places where we rejected the guidelines on questions other than the first arg label.)

Hopefully some more concrete examples can be useful in informing the discussion.

···

_____________________________

Quick context: there are things called resources. Resources can have observers. Observers are either self-owned or have an external owner object. Observers can either conform to a protocol or be closures; if the the latter, then they _must_ have an external owner (because closures aren’t objects).

There are thus three different methods to add observers to a resource — though they all cover the same underlying notion of “observer” (and in fact all boil down to the same thing internally):

    resource.addObserver(foo)

    resource.addObserver(fancyIndicator, owner: foo)

    resource.addObserver(owner: foo) {
        resource, event in
        updateStuff(resource.latestData)
    }

The API guidelines as stated would have us change that last one to:

    resource.addObserverWithOwner(foo) {
        resource, event in
        updateStuff(resource.latestData)
    }

However, this incorrectly implies that there is a distinct kind of thing that is an “observer with owner,” and that we will get one only from the third flavor of the method. That implication is wrong.

The consistency in the original of the “addObserver” name and the “owner:” label make the correct implication: all three methods serve the same purpose, observers are observers, and “owner” means the same thing in the two places it appears. I certainly think the non-compliant version of the code reads better.

There was extensive discussion around another family of methods that return a resource given either a path fragment or an absolute URL. This is where we ended up:

    service.resource("/foo")
    service.resource(absoluteURL: "http://bar.com")
    service.resource(absoluteURL: NSURL(string: "http://bar.com"))

(The first is by far the most common, the “standard” flavor.)

The guidelines would have had us do this:

    service.resourceWithPathFragment("/foo")
    service.resourceWithAbsoluteURL("http://bar.com")
    service.resourceWithAbsoluteURL(NSURL(string: "http://bar.com"))

To my eyes, this crosses the line into verbosity that impedes clarity, but there’s an even more serious problem: it wrongly implies that there’s a distinction between “resources with path fragments” and “resources with absolute URLs.” That’s dangerously wrong. One of the central conceits of the whole library is that _all_ resources get a canonicalized absolute URL, and there’s a uniqueness guarantee for that URL no matter whether it was constructed for a path fragment, an absolute URL, or resource-relative navigation.

In the cases of both addObserver(…) and service.resource(…), the guidelines would have us actively mislead users.

Another trickier example of the same issue is the much-discussed typedContent method, which downcasts based on inferred type and returns a default value if content is either missing or of the wrong type:

    var image: UIImage?
    ...
    image = imageResource.typedContent(ifNone: placeholderImage)

How to make this conform to the guidelines? The obvious fix is terrible:

    image = imageResource.typedContentIfNone(placeholderImage)

This implies … what? That the method only returns typed content if there is none of the placeholder image? No, clearly a rephrasing is necessary:

    image = imageResource.typedContentWithDefault(placeholderImage)

But again we have this problem of determining whether “with default” describes the method’s behavior, its result, or its first argument. Are we attaching content with the default somehow attached to it? (No.) Are we returning some content, or a default value if there is none? (Yes.) So maybe this is better:

    image = imageResource.typedContentOrDefault(placeholderImage)

But now my brain is having parsing problems. What is the LHS of that “or?” It just doesn’t read naturally. OK, maybe even more words can save us:

    image = imageResource.typedContentOrDefaultIfNone(placeholderImage)

Yuck. At this point, we might as well stuff the entire method abstract in the name:

    image = imageResource.typedContentOrDefaultIfNoneOrMismatchedType(placeholderImage)

Yuck squared. The original is so much clearer:

    image = imageResource.typedContent(ifNone: placeholderImage)

IMO, there’s nothing wrong with leaning on programming language syntax to help segment and clarify English syntax.

_______________________________

What’s the better guideline on first argument labels?

Radek had a nice thought in the aforementioned Gihub thread:

The rationale being, ifNone doesn't really describe the method … it describes the parameter. Most of the time, the job of the method makes the first parameter obvious (hence the guideline), but here, it doesn't. So the parameter makes sense.

I’ll give a +1 for these two recommendations from Erica, which run along the same lines as Radek’s thought, but in more thorough detail:

On Jan 23, 2016, at 6:33 PM, Erica Sadun via swift-evolution <swift-evolution@swift.org> wrote:

Prefer external names for the first parameter when the natural
semantic relationship between the parameters is stronger than their
relation to the operation.

For example, the following calls use labels for the first parameter:

login(userName: "blah", password: "...")
moveTo(x: 50.0, y: 30.0)

This example is contrary to Swift's normal naming scheme which integrates the
first argument into the function or method name, for example:

loginWithUserName("blah", password: "...")
moveToX(50.0, y: 30.0)

The coupling between x and y, username and password, (and yes it is a judgement call)
should be considered as a reason to employ an external label.

Differentiate related calls whose implementations are distinguished by their
parameters, as you would with initializers, using first parameter labels.

Instead of loginWithUserName("blah", password: "...") and loginWithCredential(myCredential),
prefer:

login(userName: "blah", password: "...")
login(credential: myCredential)

I’m not sure we’ve found the perfect, crisp way of saying all this — but I strongly agree that the existing guidelines are too rigid on the question of the first arg label, and Erica’s wording comes the closest I’ve seen to being a viable replacement.

Cheers,

Paul


(Radek Pietruszewski) #15

Ooookay, this thread took *a while* to go through. Phew!

Echoing from my reviews of SE-0005 and SE-0006:

I’m overwhelmingly *for* this proposal. I think removing needless verbosity and keeping the signal-to-noise ratio high is one of the most immediately appealing aspects of Swift, as well as a great general improvement to the programming experience.

And these guidelines do a great job at this. Like Paul Cantrell said, they recognize that both verbosity and brevity endanger clarity of code, and present many formalized rules to try to strike a balance in between, and keep S/N high. Both by reminding writers (yes, software writers!) to be explicit when it’s important, and by getting rid of noise that conveys little to no information, especially in a statically-typed language.

Although the API Design Guidelines sometimes err slightly more on the side of explicitness than me, I’m +1 aside from some concerns below:

* * *

= Prefer to follow the language’s defaults for the presence of argument labels =

This has been extensively discussed in this thread, by Erica, Paul, Dave, and others, so apologies if I repeat arguments already made, but:

I think there are more use cases where it makes sense to make the first argument label explicit than the guidelines consider.

The way I see it, most of the time, the method name, its fundamental job describes the first parameter, or makes it obvious. And so, the default behavior of Swift, and the general guideline are correct. However, there are cases, where this isn’t so.

   addObserver(foo)

Here, the method name says what first argument it takes right on the box. And `add(observer: …)` wouldn’t be appropriate, because this isn’t some general, generic “add”. The method adds an observer in particular. It’s fundamentally what it does, so it goes on the name.

   array.contains(“foo”)

This doesn’t describe the parameter explicitly, but the parameter is obvious, and makes something of a “sentence”. No label needed.

   login(username: “foo", password: “bar")

Here, “username” should _not_ be part of the name, because it doesn’t describe the fundamental job of the method. The method logs you in. Username is just a parameter.

One way to think about it, as Erica pointed out, is that the parameters are bound more strongly together than “username” is to the name. Another way to think about it, it would be completely natural to make the parameters a standalone tuple or a separate type:

   let credentials = (username: “foo”, password: “bar”)
   login(credentials)
   // or:
   login(LoginPair(username: “foo”, password: “bar”))

Another way in which it makes sense, and again this was pointed out, is that if you have multiple ways of logging in, it’s still logging in:

   login(username: “foo”, password: “bar”)
   login(token: “asdfg”)

Making the names “loginWithUsername” and “loginWithToken”:

- looks weird if you’re not used to Objective-C
- is unnecessarily verbose (tiny thing, but “with” conveys zero information, so I’d prefer to rely on Swift’s punctuation and make it an explicit parameter label)
- makes it seem as if the two methods were fundamentally different things, and they’re not. They’re two variants of the same method, so they should have a common name, and different parameters.

More examples:

   moveTo(x: Double, y: Double)

This passes multiple of the previous tests. “x” is a parameter, has nothing to do with the method itself. It could just as well be a tuple or a separate type. And if you had a different way of passing the position data, you’d still want the same name.

   constructColor(red: 0.2, green: 0.3, blue: 0.1)

Same thing.

An example from SE-0005 I proposed: https://lists.swift.org/pipermail/swift-evolution/Week-of-Mon-20160125/007658.html
   func fill(blendMode: CGBlendMode, alpha: CGFloat)
   func stroke(blendMode: CGBlendMode, alpha: CGFloat)
   func encode(coder: Coder)
Copying and pasting what I said in that thread:

Again, the method, the action itself (“fill”, “stroke”, “encode”) doesn’t naturally describe the first parameter in a way “insert” on a collection, or “addObserver” would. The blend mode and coder values here are merely _options_ of the method, not its _fundamental job_.

One way to conceptualize the difference is to think of arguments as being either “inputs” or “options”. A passed element to be inserted to a collection is an input, but blend mode is only an option of a fill, an operation that conceptually takes no inputs.

(I also don’t believe that “don’t repeat type information” rule applies here. “blend mode” is a description of the parameter, not only its type. Same with coder. We’re not needlessly repeating type information here, we’re describing option parameters, which happen to be the same as type names.)

I think the language defaults and general guidelines are good. But the guidelines should be slightly more open about other situations where overriding the default is clearer to the reader.

* * *

= Omit Needless Words =

One more example to the parameters discussion, and also related to omitting needless words:

   array.joinWithSeparator(“,”)

I have multiple problems with this API. First of all, this should be, in my mind:

   array.join(separator: “,”)

because “separator” is a parameter, not the description of the method itself.

(In fact, I would go as far as to say `array.join(“,”)` because it seems always obvious from context.)

But aside from that, I take issue with this pattern because of the word “with”. This is a needless word that should be omitted. It contains no information. It’s just syntactic glue, which is not necessary here since we can say `join(separator: …)` instead.

(A rule of thumb: If you’re tempted to write `fooWithBar(…)`, you probably should use `foo(bar:)`, just like `initWithFoo:` from ObjC is translated to `init(foo:)`)

And I see that in many places. Not much in Swift, but a lot more in Objective-C APIs. Needless words like “with”, “and”, “by”, “using”, “for". I’m not saying they’re always unnecessary; they often help convey the semantics or intent, or are needed for the method name to make sense. Or sometimes they replace a noun in describing a parameter (see: stride). But too often they’re just glue words used to make code sound like English without any actual readability benefits, and merely adding noise.

Sorry about this mini-rant. I’m not sure if this requires additional examples or clarification in the Guidelines text aside from the first-parameter discussion, but something I wanted to point out.

* * *

= If the first parameter of a method is defaulted, it should have an argument label. =

Okay, I think you all got the idea by now, but I can’t help myself. This rule says you should do something like:

   mutating func removeAll(keepingCapacity keepingCapacity: Bool = false)
   close(completionHandler completion: ((Bool) -> Void)? = nil)

But consider what would happen if these parameters weren't defaulted, for whatever reason:

It would still make more sense to say:

   removeAll(keepingCapacity: false)
   close(completionHandler: { … })

Than:

   removeAllKeepingCapacity(false)
   closeWithCompletionHandler({ … })

This suggests to me that this rule is not fundamental, rather it derives from the distinction between the implied function input and method options/parameters I discussed earlier. The use case with default values is just the most common example of this in practice, since “options” (vs inputs) very often have a sensible default.

* * *

= -ed / - ing / -inPlace =

I have nothing to add others haven’t, but I agree that these rules are still really awkward.

* * *

I haven’t found any other serious issues with the proposal. I have some ideas to _add_ to the Guidelines, but I figure those can wait and should get their own thread later. The overall language and spirit is exactly what I was hoping for.

And just don’t forget:

Good APIs aren't the result of
applying a set of mechanical rules. You have to consider what the usage
will look like.

(Dave Abrahams)

These are guidelines, not the law. It’s a great thing to have a set of guidelines applicable in 95% of cases, and have the community consistently apply them, but there’s always room for good judgement along the edges.

Best,
— Radek

···

On 22 Jan 2016, at 22:02, Douglas Gregor via swift-evolution <swift-evolution@swift.org <mailto:swift-evolution@swift.org>> wrote:

Hello Swift community,

The review of SE-0023"API Design Guidelines" begins now and runs through January 31, 2016. The proposal is available here:

https://github.com/apple/swift-evolution/blob/master/proposals/0023-api-guidelines.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
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/0023-api-guidelines.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
Thank you,

-Doug Gregor

Review Manager

_______________________________________________
swift-evolution mailing list
swift-evolution@swift.org <mailto:swift-evolution@swift.org>
https://lists.swift.org/mailman/listinfo/swift-evolution


(Erica Sadun) #16

It suddenly occurs to me it's almost the 31st and I haven't actually responded to the proposal as a proposal. I heartily endorse a general set of API guidelines and I appreciate this first approach towards satisfying that goal. I have issues with two sections.

First, I'd entirely eliminate the guidance in the "Be Grammatical" section and move the section with boolean assertions and property names to Special Instructions.

* I disagree with the mutating/non-mutating differentiation. I started writing about this here before personally abandoning it as too big a question: Grammatical.md <https://github.com/erica/SwiftStyle/blob/master/Grammatical.md>. In a nutshell, side-effects and pure functionality are not considered the way I'd want them to be and the ed/ing naming is problematic.
* The protocol question is big and wide as well, and I have written on-list: it's not "the role of a protocol to describe implementation details...Going that way leads you to over-designated hungarian-esque guidelines that I'd rather keep loose, friendly, and sensible."

In "Conventions":

* I don't like endorsing that any property could or should not be O(1) (vs recommending using a function instead)
* The case convention for enumerations remains conventional without a guiding principle. I'd rather have a firm "use lower camel case for members" that I can complain about (because it feels to me like "pick a type layout for this memory", so upper camel) rather than "use upper camel and let everyone on the Swift team complain about it" once and for all.
* With regard to argument labels, I'd like to add two rules, as detailed in-thread and written about here: ArgumentLabels.md <https://github.com/erica/SwiftStyle/blob/master/ArgumentLabels.md>

Differentiate related calls whose implementations are distinguished by their parameters, as you would with initializers, using first parameter labels. Instead of loginWithUserName("blah", password: "...") and loginWithCredential(myCredential), prefer:

login(userName: "blah", password: "...")
login(credential: myCredential)
This approach emphasizes the action being taken (login) and demotes the actual arguments involved in performing that action. In doing so, they require labels to differentiate which implementation of that action is to be used.

and

Prefer external names for the first parameter when the natural semantic relationship between the parameters is stronger than their relation to the operation.

For example, the following calls use labels for the first parameter:

login(userName: "blah", password: "...")
moveTo(x: 50.0, y: 30.0)
constructColor(red: 0.2, green: 0.3, blue: 0.1)
This example is contrary to Swift's normal naming scheme which integrates the first argument into the function or method name, for example:

loginWithUserName("blah", password: "...")
moveToX(50.0, y: 30.0)
constructColorWithRed(0.2, green: 0.3, blue: 0.1)
The relationships between (x, y), (username, password), and (red, green, blue) are strong enough to allow you to make a judgement call to employ an external label.

The following shows a counter-example.

addLineTo(p1, withWidth: 25.0)
In this call, the point and width have no natural relationship. There's no reason to create an external label for the first argument so this example follows the standard Swift call approach.

···

On Jan 22, 2016, at 2:02 PM, Douglas Gregor via swift-evolution <swift-evolution@swift.org> wrote:

Hello Swift community,

The review of SE-0023"API Design Guidelines" begins now and runs through January 31, 2016. The proposal is available here:

https://github.com/apple/swift-evolution/blob/master/proposals/0023-api-guidelines.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
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/0023-api-guidelines.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
Thank you,

-Doug Gregor

Review Manager

_______________________________________________
swift-evolution mailing list
swift-evolution@swift.org
https://lists.swift.org/mailman/listinfo/swift-evolution


(Patrick Gili) #17

My evaluation is inline below...

Hello Swift community,

The review of SE-0023"API Design Guidelines" begins now and runs through January 31, 2016. The proposal is available here:

https://github.com/apple/swift-evolution/blob/master/proposals/0023-api-guidelines.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
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/0023-api-guidelines.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?

Overall, I like the proposal. It provides sound guidance that can lead to consistent code in practice. However, there are some issues and concerns I have:

- Under "Fundamentals", there is a bullet called "Write a documentation comment...". Under this bullet, this is another bullet called "Use a single sentence fragment...". Why? I find sentence fragments often detract from clarity and concise nature, which can lead to confusion.

- Under "Naming", there is a bullet called "Omit Needless Words". This bullet states, "Occasionally, repeating information is necessary to avoid ambiguity..." It would be useful to provide an example that the reader can use to disambiguate this use case.

- Under "Naming", there is a bullet called "Compensate for Weak Type Information...". The example provided is confusing. First, it contradicts the guidance relating to omitting needless words. It suggests that

func add(observer: NSObject, for keyPath: String)

becomes

func addObserver(_ observer: NSObject, forKeyPath: String)

This results in "Observer" followed by "observer". Why is this more clear?

In addition, I don't understand why it collapsed "for keyPath" to "forKeyPath". Perhaps an explanation would clarify?

- Under "Naming", there is a bullet called "Uses of non-mutating methods should read as noun phrases...". This bullet provides an example of an exception. Why would calling this method firstAndLast() have less clarity than split()? Perhaps a better example is in order.

- Under "Naming", there is a bullet called "When a mutating method is described by a very, name its non-mutating counterpart...". On the surface this appears to provide good guidance, I think in practice it becomes difficult and doesn't necessarily provide the desired result. I think Ruby provides a better example of how the desired result is very clear. In Ruby, the name of a mutating method is designated by an exclamation point at the end of the name. For example, myString.reverse() returns the value of the reversed string, while myString.reverse!() mutates myString with the reversed value. I'm not necessarily proposing that Swift use the exclamation point for this purpose (as Swift already uses this force unwrapping), so much as I'm proposing that you investigate how other languages clearly disambiguate non-mutating and mutating methods.

- Under "Conventions", there is a bullet called "Methods can share a base name when they share the same basic meaning...". There are some examples when this convention does not apply. I think it would be helpful to illustrate to the reader how to address these exceptions (i.e., do this, instead of that).

- Under "Conventions", there is a bullet called "Prefer to follow the language's defaults for the presence of argument labels". Be aware that the current compiler issues the warning when the first argument label is "_", "Extraneous '_' in parameter: <parameter> has no keyword argument name". I would either like the compiler to not issue a warning for this use case, or at least give the developer the means to disable this warning.

Is the problem being addressed significant enough to warrant a change to Swift?

I think publishing a clear set of guidelines is absolutely necessary.

Does this proposal fit well with the feel and direction of Swift?

Absolutely. Great job!

If you have used other languages or libraries with a similar feature, how do you feel that this proposal compares to those?

I think all serious languages have a document that specifies a set of guidelines with the intent of bringing consistency in the use of the language. Languages that do not drive a set of best practices tend to suffer, at least form a readability perspective. For as much as I like Ruby, it lacks such a set of guidelines, which has resulted in the community writing code that lacks the consistency that makes it easy to read others' code.

How much effort did you put into your review? A glance, a quick reading, or an in-depth study?

As always, I read the proposal multiple times, which included reading through the guidelines multiple times, sleeping on it, pondering it, and finally writing my evaluation.

···

On Jan 22, 2016, at 4:02 PM, Douglas Gregor <dgregor@apple.com> wrote:

More information about the Swift evolution process is available at

https://github.com/apple/swift-evolution/blob/master/process.md
Thank you,

-Doug Gregor

Review Manager

_______________________________________________
swift-evolution-announce mailing list
swift-evolution-announce@swift.org
https://lists.swift.org/mailman/listinfo/swift-evolution-announce


(Joseph Lord) #18

Overall I like the proposals but I do have comments on a few issues.

enum case capitalisation. I generally like the use of lowerCamelCase names because they are not types. Types are determined at compile time and the case (value) of an enum is only known at runtime. I have already seen people get confused by expecting to be able to use information about an enum value at compile time. I think normally using upperCamelCase would increase the confusion between cases and types.

I have flicked through the thread and there are some good interesting discussions particularly about argument labels in different ways. I'm not sure there are any specific changes I want to strongly back but the current proposals are not the uniquely correct approach. If there are substantial changes it would be good to run them through as a proposal for changes on top of this one so that they get proper review, the thread is huge and many people may have missed things that they really care about.

Joseph

···

On 22/01/2016 21:02, Douglas Gregor via swift-evolution wrote:

Hello Swift community,

The review of SE-0023"API Design Guidelines" begins now and runs through
January 31, 2016. The proposal is available here:

    https://github.com/apple/swift-evolution/blob/master/proposals/0023-api-guidelines.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

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/0023-api-guidelines.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

Thank you,

-Doug Gregor

Review Manager

_______________________________________________
swift-evolution mailing list
swift-evolution@swift.org
https://lists.swift.org/mailman/listinfo/swift-evolution

--
Human Friendly Ltd.


(Colin Cornaby) #19

I’m still processing all my thoughts on this and might have a thought or two on the Obj-C bridging thread as well, but I had a question about one specific style guideline:

Uses <> of nonmutating Boolean methods and properties should read as assertions about the receiver, e.g. x.isEmpty, line1.intersects(line2)

I have really mixed feelings on this, and was wondering what the rationale was (maybe to convince me it’s a good idea.) To me, this seems like adding extra language to a proposal that is pretty good at removing now unnecessary language. It also seems unhelpful to me as an API user and writer because in an alphabetical list, related functionality (like a “playing” nonmutating property and a “play” function, or a “playRate” mutable property) are all going to be moved away from each other in a list. I acknowledge that Xcode’s autocomplete is getting better though in that regard, and that most documentation is grouped. But I’m not a fan of the crowding of “is” functions that is going to happen.

In reviewing AVFoundation specifically (but I’m going through the others as well), I’m not sure this is an improvement either.

Mildly worse, makes it sound like a function instead of a property:
exposurePointOfInterestSupported -> isExposurePointOfInterestSupported

Same sort of thing, I’m not sure this is cleaner:
supportsVideoMirroring -> isVideoMirroringSupported

This one almost sounds to me like the function meaning is being tweaked:
videoMirrored -> isVideoMirrored

The name of this one now sounds like it is supposed to have an argument to check to see if a specific video orientation is supported (although it is true that the original property wasn’t named great either):
supportsVideoOrientation -> isVideoOrientationSupported

Like I said, I’d be willing to be talked into this, but at this point I’m just really curious what the justification is. For simple cases this seems to work well, but I’m really not enthused for what this does to more complex or domain specific property names. I’m willing to call this “unease after doing Obj-C too long”, but I’d like to understand why the Swift team feels this is better.

···

On Jan 22, 2016, at 1:02 PM, Douglas Gregor via swift-evolution <swift-evolution@swift.org> wrote:

Hello Swift community,

The review of SE-0023"API Design Guidelines" begins now and runs through January 31, 2016. The proposal is available here:

https://github.com/apple/swift-evolution/blob/master/proposals/0023-api-guidelines.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
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/0023-api-guidelines.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
Thank you,

-Doug Gregor

Review Manager

_______________________________________________
swift-evolution mailing list
swift-evolution@swift.org
https://lists.swift.org/mailman/listinfo/swift-evolution


(Janosch Hildebrand) #20

I'd like to add my voice to the many that are in favor of this proposal.

I agree with the general spirit of the guidelines and I think they cover the most important points. They can always be expanded in the future if experience deems it necessary but I'd rather have people actually read and use the document than be put off by length and complexity.

Various minor points

* Will these guidelines become part of "The Swift Programming Language"? Personally I would support that.

* I'm in favor of lowerCaseEnumCases

* var htmlElement = HTMLElement

* I don't think stripping the "Type" suffix from protocols is a clear win, mostly a change of tradeoffs (which have already been discussed extensively).
Ultimately I would be fine with either approach.

* One small idea I'd like to throw out there is whether the guidelines should contain a small note that one might look to the standard library for inspiration as well. It will have many examples for following the guidelines as presented and might offer some helpful precedent in cases where one is still unsure. In a way this is probably obvious but it might not hurt to mention?

* On the guidelines page the bullet point "When the first argument is defaulted, it should have a distinct argument label." is wrapped in a link (without a target). This is probably unintentional.

And a bit of rambling regarding the property vs method discussion:
The current situation seems to be that there are a lot of conflicting "rules" (most with exceptions attached) that need to be weighed against each other, with the decision coming down to "collective gut feeling". It don't see a way to formalize them without breaking at least some existing conventions and probably some heated discussions ;-). I also wonder if that would actually produce better APIs on the whole or simply produce clear rules for the sake of having clear rules with APIs suffering in some cases...

- Janosch