[swift-evolution] [Review] SE-0023 API Design Guidelines

Radosław Pietruszewski radexpl at gmail.com
Mon Jan 25 12:07:41 CST 2016 

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 <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 at swift.org <mailto:swift-evolution at 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 <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 <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 <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 <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

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

More information about the swift-evolution mailing list