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

[David Owens II]

> On Jan 24, 2016, at 11:17 AM, Dave Abrahams via swift-evolution <swift-evolution at swift.org> wrote:
> 
> on Sat Jan 23 2016, David Owens II <swift-evolution at swift.org> wrote:
> 
>> I think Joe's reply is a pretty good summary.
> 
> Hi David,
> 
> Let me start by expressing my appreciation for the way you're struggling
> with the hard issues here.  The questions you're asking reflect many of
> the same ones we asked ourselves during the development of these
> guidelines.
> 
>> At a high-level, I really get no sense on how APIs are really supposed
>> to be developed in Swift. Joe talks about second arguments generally
>> becoming prepositional phrases, but is that the true intent? Because
>> that's not what the guideline says, nor what the language semantics
>> really promote or suggest to do.
> 
> No, that's not the intent of the guideline.

"The use of external parameter names can allow a function to be called in an expressive, sentence-like manner, while still providing a function body that is readable and clear in intent."

Source: https://developer.apple.com/library/ios/documentation/Swift/Conceptual/Swift_Programming_Language/Functions.html

That's the 2.1 guide, so your mileage may vary in 3.0, but that's some of the baggage coming in to this review.

The problem I see is that there is a mixed bag of what "good" APIs look like. And when push comes to shove, code wins. The libraries exposed in code, even through the ObjC bridging, will be used as the basis of what determines what good Swift APIs look like more than a published doc.

However, what I see is the struggle between creating good API call sites, what the defaults of the language provide today, and what the guidelines are saying and comparing those to what we get from the new Swift APIs and the updated ObjC imported APIs.

By default we get this:

func max(a: Int, b: Int) -> Int
max(a, b: 12)

func addObserver(observer: Observable, path: String)
thing.addObserver(observing, path: "/path/to/observe")

public func strideTo(end: Self, stride: Stride) -> StrideThrough<Self>
thing.strideTo(10, stride: 1)

Of the three, only the addObserver are APIs I deem to be acceptable. Of course, there are many examples to prove any given point, but I think these represent the common cases.

1. Labels are obviously bad (max)
2. Labels are fine though the external label could be better (addObserver)
3. Exposes an implementation name that really confuses the API (strideTo)

(more on this example a bit later)


>> For example, the guidelines say to do one thing but the example does
>> something different:
>> 
>>> 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)
>>> 
>>> 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
>> 
>> This example already had a "for" in the label, but if it were not
>> already there, the API, according to the guidelines, should become
>> this:
>> 
>> func addObserver(_ observer: NSObject, keyPath path: String)
>> grid.addObserver(self, keyPath: graphics) // clear
> 
> The reason "for" is there is that otherwise you can't tell what the
> relationship between the observer and the keyPath is.  For example, if
> it had been "at" instead of "for," it would completely change the
> meaning. "Of" would probably be more descriptive, frankly.  But the
> example isn't trying to illustrate anything about that preposition or
> that relationship.  

Right, but the argument is that the API is indeed better with the "for" preposition. I believe it is, I think you are also saying that it is.

This is the closest guideline that I think attempts to address it:

> Clarity at the point of use is your most important goal. Code is read far more than it is written.

But the clarity here is, ironically, inherently ambiguous. Clarity could mean to make the weak type information known. Clarity could be about intention of how the parameter could be used. Clarity could simply be a more verbose name that provides additional context. I think it would be clear to have guidelines that actually describe what you think make up a good call site.

thing.strideTo(10, stride: 1) // clear
thing.strideTo(10, by: 1)     // clear and linguistically better
thing.strideTo(10, 1)         // not clear

Both provide some amount clarity at the call site as the value of 1 is quite clear what it means. So in one sense, I agree that the defaults of the language today push you towards clarity.

To me, the guidelines, as I understand them, lead us to the first option when I believe the intent is to actually get to the second usage.

>> Especially when compared with the ObjC import items. Seemingly, the
>> ObjC APIs would be using prepositional clauses for parameter labels
>> while the Swift labels would simply be more descriptive nouns.
> 
> I don't know what you mean here, sorry.

Many of the ObjC APIs will come across with prepositions in the external labels, such as:

func insert(_ anObject: AnyObject, atIndex index: Int)

Whereas my interpretation of the Swift 3.0 guidelines would say to design that API like this:

func insert(_ anObject: AnyObject, index: Int)

So when we look at the various API options in front of us when designing new APIs, do we use "atIndex", "at", or simply stick with the default "index"? This is where I think the guidelines don't help steer us in any direction.

However, indeed someone thinks that we should use "at", but based on the guidelines, it's really hard to understand why:

-  mutating func insert(newElement: Iterator.Element, atIndex i: Int)
+  mutating func insert(newElement: Iterator.Element, at i: Int)

Again, I agree that "at" reads better at the call site. However, it doesn't provide any real clarity over the default "index" (sidenote: i is still a bad choice ;)), and in some ways, it could be argued that it's slightly ambiguous. Does the below mean insert "12" at the index of "2" or at the value matching "2"?

items.insert(12, at: 2)

While this version does not even have that ambiguity. This also relates to the "weak type information" guidelines.

items.insert(12, atIndex: 2)


>> The rules I described are keeping in strict guidance to the API design
>> guidelines of adding nouns. So the above would actually be:
>> 
>> func add(observer o: NSObject, keyPath path: String)
>> grid.add(observer: self, keyPath: graphics)
> 
> I'll have to go back and look at the rules you described, but regarding
> the above example, my reason to keep "observer" in the base name is that
> adding observers is really a totally distinct thing from adding, say,
> gesture recognizers, or animation steps, or anything else.  There isn't
> one big pot to which you're adding everything.  If there were, the
> receiver would be like a collection, and we wouldn't need a noun there
> at all.

Granted, this is the case some of the times. However, if the protocol is this:

protocol Observable {
    func add(observable o: NSObject, forKeyPath path: String)
    func remove(observable o: NSObject)
}


I actually really hard pressed to come up with a definitive example as to why that should be allowed. The best I can argue is that "add" itself is too generic and could be confused on the implementing type that may also have an "add" function. However, the additional label information helps with that.

Also, the argument against this one:

protocol Observable {
    func addObservable(o: NSObject, forKeyPath path: String)
    func removeObservable(o: NSObject)
}

Would seem to be me that we are simply duplicating type information. Maybe it's more clear?

In any respect, I at least submit that "addObservable" in this case is definitely in the grey area.

However, I absolutely believe that the "strideTo/strideThrough" example is clearly on the side of really being part of the argument label. In both cases we are striding, the difference is on where we stop.

extension Strideable {

-  public func stride(to end: Self, by stride: Stride) -> StrideTo<Self>
+  public func strideTo(end: Self, by stride: Stride) -> StrideTo<Self>

 }

 extension Strideable {

-  public func stride(through end: Self, by stride: Stride) -> StrideThrough<Self>
+  public func strideThrough(end: Self, by stride: Stride) -> StrideThrough<Self>

 }

This change seems to be simply because the usage of labelled first parameters are frowned up, not because it's actually a better place to describe what is going on.

>> However, if the API guidelines are really about creating APIs that are
>> to be read in a more English-like manner, like ObjC APIs are designed,
>> then the guidelines should really be amended to make that clear.
> 
> The point of the guidelines is not to make code look like English; it's
> to make code clear and understandable.  It happens that in many cases,
> leveraging English grammar serves that purpose well.  In other cases
> it's necessary to take a different approach, which is why we have sin(x)
> and zip(x, y).  But as I think we make clear, one shouldn't add words
> just to make an API more “English-like.”
> 
>> If the intention is truly that Swift APIs are supposed to read as
>> naturally as ObjC APIs, 
> 
> Ah... “naturally” is in the eye of the beholder, though.

Not quite, I think we can objectively say that this:

items.insert(12, atIndex: 2)

Reads far more naturally than this:

items.insert(12, index: 2)

In the later, you have to add words to make the API read fluidly. The former simply does by the nature of the words.

>> then I completely agree with Joe that there should be a language
>> change to actually require the label for parameters.
>> 
>> func addObserver(o: NSObject, path: String) // error: Argument label required for `path`.
> 
> I'm confused; "path" has an argument label; it's "path."

What I'm saying is that this is a compiler error:

func addObserver(o: NSObject, path: String) // error: Argument label required for `path`.

While this version is not:

func addObserver(o: NSObject, forKeyPath path: String) // error: Argument label required for `path`.

So the external API label is not simply defaulted to the name of the variable within the function scope.

>> All of my inclinations for C-style syntactical languages say the API
>> should be (in order of my conceptual preference model):
>> 
>> grid.add(observer: self, keyPath: graphics)
>> 
>> // or
>> 
>> grid.addObserver(self, keyPath: graphics)
>> 
>> // or
>> 
>> grid.addObserver(self, graphics)
> 
> I think we're asking you to let go of your inclinations and think more
> carefully about what will serve the reader of code using your API :-).
> This is less about having a pattern to follow than about how to make
> choices that lead to readable, maintainable code.

I'm trying to, but we're not really following the ObjC guidelines either. At least on paper. I think you keep talking about readable APIs, but I feel the guidelines keep talking more about descriptive. The difference is subtle, I agree. But I think it's the difference between these two APIs:

items.insert(12, atIndex: 2)  // or items.insert(12, at: 2)

items.insert(12, index: 2)

Again, it's about which is supposed to be "canonical" Swift.

-David

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