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

[Dave Abrahams]

on Sun Jan 24 2016, David Owens II <david-AT-owensd.io> wrote:

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

Understood; the "sentence-like" idea keeps getting repeated.  The
keepers of Cocoa APIs tell me that, since several years ago, they moved
away from adding prepositions to secondary selector pieces to make
things more sentence-like.

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

Yep.  But a big part of the reason to publish the doc is so we can
decide what direction to push the library APIs.

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

IIUC you'd prefer to see an initial argument label on the stride
function?  I get that, but can you explain why you consider the
alternative to be *unacceptable*?  Even below, where you talk about why
it might be better to use a label, you don't really make the case for
why we shouldn't tolerate not using one.

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

Covered by the guidelines.

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

Yes.  So... what are we arguing about?

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

We intend that to mean "clarity of semantics."  Would that change, um,
clear this up for you?

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

Actually I think the first is unclear simply because the word "stride"
takes two different roles, one as a noun and one as a verb.  It's
possible to misinterpret the 1 as being an expression of the word in the
base name, or at least it's possible to be confused by that until you
work out all the reasons that it doesn't make sense.

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

Yes, IMO that is the intent!  But why do you think they lead us to the
first option?  Because they explicitly mention using nouns to clarify
roles?

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

That'll come in as

  func insert(_ anObject: AnyObject, at 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)

That's not the intent; the intent would be to move you toward what I
wrote above.

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

Fair point; we could be more explicit about that.  I think the
admonition to avoid merely repeating type information is the applicable
one here, but it really only works to push you away from "index" in the
context of an associated Index type, and NSArray doesn't have one.  You
can see the idea expressed in the importer rules at
https://github.com/apple/swift-evolution/blob/master/proposals/0005-objective-c-name-translation.md

,----
| Index in the selector piece matches Int in the type name:
| 
| func characterAtIndex(_: Int) -> unichar
|                 ~~~~~    ~~~
`----

> 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 ;)), 

It all depends on what it does for the doc comment.  IMO there's nothing
wrong with “Inserts `newElement` before the `i`th element, or at the end
if `i == count`”.

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

Yes.  This all becomes a little bit more complicated when you consider
fitting it into code with generics, because you can't know when an
associated type might have a broader role.

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

[don't you mean "observer o:" above?]

> }
>
> 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.

You have to consider how this protocol is going to be used; it's a
mix-in that will be broadly applied to many things that could add an
NSObject for many purposes.  Unfortunately, as hard as we tried, we were
unable to come up with guidelines that actually make API design easy!

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

Where?  There's nothing about NSObject that implies it's an observer.  I
could see that argument with

  protocol Observable {
      func addObserver(o: Observer, forKeyPath path: String)
      func removeObserver(o: Observer)
  }

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

I can't argue with you here.

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

Agreed.

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

OK.

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


OK, I'll try to come up with some wording adjustments that sort this out.

Thanks for your patience,

-- 
-Dave