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

[Dave Abrahams]

on Sun Jan 24 2016, Paul Cantrell <cantrell-AT-pobox.com> wrote:

>> On Jan 24, 2016, at 7:25 PM, Dave Abrahams via swift-evolution
>> <swift-evolution at swift.org> wrote:
>> 
>> 
>> on Sun Jan 24 2016, Paul Cantrell <swift-evolution at swift.org> wrote:
>> 
>>> 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.
>> 
>> They've been vetted against all of Cocoa (to the best of our
>> ability—it's huge) and against a couple of sample projects, Lister
>> and DemoBots.  This was a major effort and we endeavored to cover all
>> the bases.
>
> Well, my mistake then! Always dangerous to speculate on the thoughts
> and experiences of other people.
>
>>>    resource.addObserver(foo)
>>> 
>>>    resource.addObserver(fancyIndicator, owner: foo)
>>> 
>>>    resource.addObserver(owner: foo) {
>>>        resource, event in
>>>        updateStuff(resource.latestData)
>>>    }
>> 
>> Oh, that's an interesting API.  I think it's pretty unusual that the
>> direct object of the method is a trailing closure, and IMO inventing
>> some conventions for handling this kind of API in your own code would be
>> perfectly appropriate.
>
> Yes, “direct object is a trailing closure” is a very good way of
> summarizing the problem here. And I did in fact think about making the
> closure the first arg, but the obvious hideous consequences when the
> closure gets long steered me away from that. Swift is right to give
> special deference to trailing closures, IMO.
>
> You may have hit on an underlying principle with your “direct object
> of the method” observation: many of the cases we’re talking about
> where the first arg wants a label are ones where the method is a verb
> and its direct object is not the first arg alone, either because (1)
> the direct object is something other than the first arg, (2) the
> direct object is several args taken together, or (3) the verb is not
> transitive.
>
> Could that lead to an improvement in the guidelines?

I think it's very promising; we'll have to see how well it flies.

> Case (1) covers the trailing closure above, and also typedContent
> below, whose direct object is implicit. Case (2) covers moveTo(x:y:)
> and arguably also covers logInWith(username:password:). (Aside:
> “login” is a noun and “log in” is a verb, dang it, and the method
> should be “logIn” … but we never do that because it looks
> ridiculous. Grammar…!) Case (3) covers dismissAnimated.

Nice!


>> Yes, one of the complaints I have about the guidelines trying so hard to
>> avoid initial argument labels is that it causes some things to be read
>> as closely-associated when they should not be.
>
> You’ve hit the nail on the head there. Decision-makers take note!
> Gratuitous Unicode for emphasis! ✮✮✮✮✮✮
>
>> Others developing the API guidelines felt that the parenthesis was not
>> significant enough to be meaningful to people reading the call, and that
>> any increased expressivity was not paid for by the fact that the surface
>> of our APIs would be less-consistent (a more even balance of APIs with
>> and without initial labels is less consistent than having initial labels
>> be quite rare).
>
> I don’t think this is the case. The paren seems significant to my eyes
> — more and more so the less I grow accustomed to Obj-C, with all its
> syntactic noise — and we have ever-better autocomplete, compiler
> fixits, and online docs to help discover when initial args are needed.
>
>>>    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.)
>> 
>> Sorry, perhaps it should be obvious, but why do you need the label at
>> all here?  Seems to me you could do this with two overloads, since it's
>> easy to detect when a string is an absolute URL.
>
> An excellent question, and I’ll try to answer it without going too far
> off topic:
>
> It’s a security feature. Because of how the first flavor (path
> fragment) works, it’s likely that unsanitized user input will creep
> into it, e.g. "/user/\(userTextField.text)". (This is doubly true of
> its counterpart resource.child(…).) Auto-detecting absolute URLs would
> be dangerous: it could cause problems such as leaking credentials or
> posting sensitive data to a malicious server. The flavor that takes
> absolute URLs, on the other hand, is typically traversing intra-API
> hrefs returned by the API itself. I wanted to keep those use cases
> well separated.

I'll take your word for it; it definitely sounds like you know what
you're doing in this area.

> I considered disallowing the second flavor (absolute URL string), and
> distinguishing the remaining two on a string vs. NSURL basis. However,
> having an absolute URL string is a far more common case than having an
> NSURL, and letting the library do the string → NSURL conversion lets
> it produce more useful debugging / diagnostic info for malformed URLs.
>
>>> Yuck squared. The original is so much clearer:
>>> 
>>>    image = imageResource.typedContent(ifNone: placeholderImage)
>> 
>> Point taken, but if you let go of determining the return type by type
>> inference (pretty close to overloading on return type) you might be
>> better off.
>> 
>>      i = Image(foundIn: resource, default: placeholder)
>
> That’s a really nice thought, and a reminder that I’m still thinking
> inside the boxes of other languages.
>
>>> 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.
>> 
>> I have a few problems with the above wording, but I'm all for loosening
>> the restriction.
>
> Erica’s wording isn’t perfect, but I feel like it’s getting
> warm. Certainly the best effort at a specific revision I’ve seen.
>
>> the challenge is convincing the people that matter that the
>> increased expressivity of allowing first argument labels in more places
>> is worth the increased non-uniformity of APIs.
>
> Consistency is always in tension with expressivity, and it’s a hard
> balance to strike.
>
> The problem for those of us outside Apple is not that we’re bound by
> the guidelines; it’s that we aren’t. If the guidelines get in the way
> too much, people will just ignore them, and they’ll lose their
> value. If they’re too loose, they lose their value. It’s a tricky
> middle ground.

Don't worry, we're aware of these risks.  We want these guidelines to be
widely adopted outside Apple and lead to more overall uniformity in how
Swift code is written: that's good for everybody concerned.  If they
aren't acceptable to our customers, we'll have failed.

-- 
-Dave