[swift-evolution] When to use argument labels, part DEUX

[Dave Abrahams]

on Thu Feb 11 2016, Radosław Pietruszewski <radexpl-AT-gmail.com> wrote:

>>> 	private func setHandoffState(hash hash: String, fallbackURL: String, title: String? = nil, searchable: Bool) {
>>> 
>>> This seems OK to me, but I’m not entirely sure if the guidelines
>>> suggest I _should_ put a preposition there, i.e.
>>> 
>>> 	setHandoffStateFor(hash: a, fallbackURL: b, …)
>>> 
>>> I don’t think I should — I don’t see how “for” adds to clarity here,
>> 
>> Me neither, but it's hard to tell what this method is supposed to mean.
>> Maybe you could explain (or show a beautifully crafted doc comment
>> (BCDC) per
>> <https://swift.org/documentation/api-design-guidelines/#write-doc-comment
>> <https://swift.org/documentation/api-design-guidelines/#write-doc-comment>>
>> ;-)
>
> Ahh, but I’m missing BCDCs in many of these places ;)
>
> This is a helper that essentially makes a NSUserActivity from passed
> parameters and calls `becomeCurrent` on it. (Pretty much changes
> application-global state.) (Not the most elegant solution, I know.)

I don't know whether it's elegant, but given how you described it, I
would probably call it something like:

   createCurrentActivity(hash: a, fallbackURL: b, …)
>> 
>>> but I want to make sure if I interpret the proposed guidelines right.
>>> 
>>> 	func actionForCommand(command: UIKeyCommand) -> String? {
>>> 
>>> This should become “actionFor(command: c)” — which isn’t an obvious
>> 
>> That looks like a call, in which case the API above doesn't allow an
>> argument label.
>
> I’m sorry — I got confused — what do you mean here?

Sorry, I didn't notice that you both transitioned from declaring
(actionForCommand(_)) to calling (actionFor(command:)) *and* changed the
API at the same time.

So, I would say the guidelines say the call should look like:

    b.action(for: c)

>>  (I think this kind of confusion is probably the
>> strongest reason for changing the default---I see it come up over and
>> over).
>> 
>>> win, but I certainly don’t mind it.
>> 
>> according to where we're currently headed (prepositions inside the
>> parens), it would be:
>> 
>>    func action(for command: UIKeyCommand) -> String? {
>> 
>>> 	private func createAttribute(tag: SecItemAttr, _ data: NSString?) -> SecKeychainAttribute? {
>>> 
>>> Forgive my barbaric removal of external labels. 
>> 
>> Nit: “argument labels,” please. 
>
> Noted.
>
>> 
>>> It should be `createAttribute(tag: t, data: d)`. 
>> 
>> Again that looks like a call, so for the above declaration it would be
>> 
>>      createAttribute(t, d)
>> 
>>> Alternatively, it could be `attributeFor(tag: t, data: d)` — I could
>>> go either way.
>> 
>> Why “for?” Can you explain the semantics of this method (or show a BCDC)?
>
> This is a initializer-like helper method that instantiates a
> SecKeychainAttribute from passed parameters.

If it's a factory, I think guidelines prescribe the call should look like:

   makeAttribute(tag: t, data: d)

>>> (adding to my thoughts on the later proposal of moving preposition
>>> inside parens, the symmetry in param labels would likely be broken
>>> here, because it doesn’t seem necessary to add “for”/“with” when I go
>>> with “createAttribute”, but for the noun-only version it would have to
>>> be “attribute(forTag: t, data: d)”.)
>>> 
>>> 	func attachmentForImage(image: UIImage) throws -> Attachment {
>>> 	func attachmentForData(data: NSData) -> Attachment {
>>> 	func attachmentForFileURL(url: NSURL) throws -> Attachment {
>>> 
>>> This should be `attachmentFor(image:)`, `attachmentFor(data:)`, and
>>> `attachmentFor(fileURL:)`. 
>> 
>> again, the latest draft puts prepositions inside the parens.
>
> Q: For the previous example you suggested the latest draft would be
> `action(for:)`. Given these methods take one of three different
> parameter types, and the names mostly just repeat type information,
> should they all be `attachment(for:)`? Or `attachment(forImage:)` etc?
> The third method is trickier because it takes an NSURL, but there is
> an assumption this is a _file_ (i.e. device-local) URL.
>
>>  But without the BCDC it's pretty hard to tell whether these are good
>> names.
>
> Also an initializer-like helper method that transforms passed
> parameters into an Attachment.

Oh, definitely “makeAttachment(x)” at the call site, then, if you can't
just do this with an Attachment initializer!

>>> This seems like a win, since related methods for different data types
>>> are grouped more tightly together. (I could also — like you suggested
>>> about not optimizing for method families — make an enum, but it would
>>> be overkill since it’s not API for public consumption. I should mark
>>> those as private.)
>> 
>> I'm not trying to say “method families are bad.”  I'm just saying I
>> don't want to build the guidelines around them if possible.
>> 
>>> 	private func messageCell(label label: String) -> MessageCell {
>>> 	private func cellForTask(task: Task, enabled: Bool = true) -> TaskCell {
>>> 
>>> Hm, those are inconsistent. I could go with `cellFor(message:)` and
>> 
>> don't you mean cellFor(label:) ?
>> 
>>> 
>>> `cellFor(task:)`, which would be consistent with the guidelines. OTOH,
>>> since they return different types, I liked that they had different
>>> names… `taskCellFor(task:)` is redundant. Perhaps `taskCellFor(_:)`
>>> and `messageCellFor(_:)`.
>>> 
>>> 	func generate(fill: Double, text: String) -> CLKComplicationTemplate {
>>> 
>>> `templateFor(fill:, text:)`
>>> 
>>> 	func update(from old: TaskCommentRowModel?, to new: TaskCommentRowModel) {
>>> 
>>> That’s the, IMO rare, circumstance where the preposition *does* make sense to go inside the parens.
>>> 
>>> * * *
>>> 
>>> I mentioned this in some other post before, but I also found a bunch
>>> of methods where the first parameter should be labeled, but isn’t,
>>> because without an easy shortcut like the old #param, I was just too
>>> lazy to do The Right Thing.
>>> 
>>> PS. If someone wanted to do the same thing and review methods in their
>>> project, here’s the quick&dirty Ruby script for this:
>>> https://gist.github.com/radex/b425c73afde84d88e4ca
>>> <https://gist.github.com/radex/b425c73afde84d88e4ca <https://gist.github.com/radex/b425c73afde84d88e4ca>>
>>> 
>>> HTH,
>>> — Radek
>>> 
>>>> On 08 Feb 2016, at 21:55, Dave Abrahams via swift-evolution
>>>> <swift-evolution at swift.org> wrote:
>>>> 
>>>> 
>>>> on Mon Feb 08 2016, Radosław Pietruszewski <swift-evolution at swift.org> wrote:
>>>> 
>>>>> Dave,
>>>>> 
>>>>> First of all, thank you for enduring our nitpicks and complaints and
>>>>> continuing to explore the subject :) I think we’re all better off for
>>>>> it, and getting closer to the solution with each iteration.
>>>>> 
>>>>> You asked:
>>>>> 
>>>>>> 1. I'm not expecting these guidelines to make everybody optimally happy,
>>>>>> all the time, but they shouldn't be harmful.  Are there any cases for
>>>>>> which they produce results you couldn't live with?
>>>>> 
>>>>> And I think, by this standard, the guidelines you proposed seem to be
>>>>> a success. Looking through Doug’s diffs, I see a lot of method names
>>>>> that I don’t *love*, but I couldn’t find something I would hate.
>>>> 
>>>> That's great news!  Keep in mind, though, that Doug's results are just
>>>> the result of trying to approximate application of the guidelines by
>>>> using heuristics.  To do a complete evaluation also need to think about
>>>> what applying the guidelines will do to your own code.
>>>> 
>>>> Thanks,
>>>> 
>>>> -- 
>>>> -Dave
>>>> 
>>>> _______________________________________________
>>>> swift-evolution mailing list
>>>> swift-evolution at swift.org
>>>> https://lists.swift.org/mailman/listinfo/swift-evolution
>>> 
>>> _______________________________________________
>>> swift-evolution mailing list
>>> swift-evolution at swift.org <mailto:swift-evolution at swift.org>
>>> https://lists.swift.org/mailman/listinfo/swift-evolution <https://lists.swift.org/mailman/listinfo/swift-evolution>
>> 
>> -- 
>> -Dave
>> 
>> _______________________________________________
>> swift-evolution mailing list
>> swift-evolution at swift.org <mailto:swift-evolution at swift.org>
>> https://lists.swift.org/mailman/listinfo/swift-evolution <https://lists.swift.org/mailman/listinfo/swift-evolution>

-- 
-Dave