[swift-evolution] [Review] SE-0005 Better Translation of Objective-C APIs Into Swift

[Dave Abrahams]

on Tue Feb 02 2016, Radosław Pietruszewski <swift-evolution at swift.org> wrote:

>>> Ah, interesting!
>>> 
>>> I definitely see the rationale for this. Calling a method like
>>> `tracks` seems a bit confusing, it doesn’t capture the intent at
>>> all. The ObjC-convention version, say, `tracksWithMediaType(…)`,
>
>>> though less clear, makes a better job at this.
>>> 
>>> I can see more methods of this kind in the diff, and they seem to benefit the most.
>>> 
>>> I mentioned this before, but the way I would prefer this named is
>>> `findTracks(…)`, and skip the “with” in the name. The intent is
>>> captured better than the original, we start with a verb, and the
>>> method name is separated from its parameters. But obviously this is
>>> unlikely to work as an automated translation.
>>> 
>>> Having said that, a lot of the changes seem like a step back:
>>> 
>>> 
>>>>    func highlight(level val: CGFloat) -> NSColor?  func
>>>> highlight(level val: CGFloat) -> NSColor?
>>>> - func shadow(level val: CGFloat) -> NSColor?  + func
>>>> shadowWithLevel(val: CGFloat) -> NSColor?
>>> 
>>> Inconsistency. Highlight analyzed as as a verb, shadow as a noun,
>>> even though those are obviously related.
>> 
>> Yes, this is an inconsistency we would need to deal with via
>> NS_SWIFT_NAME. “highlight” is acting as an adjective here, although
>> it’s predominantly a verb in Cocoa APIs.
>
> True, but it would seem preferable to me to choose an approach for
> which having to fix up a name with NS_SWIFT_NAME is a rare occurrence.

The basic philosophy here is that we want to improve Cocoa's conformance
with the guidelines while doing as little damage as possible to names
that might be well-chosen.  It's sort of a “trust humans first”
approach.  That means we very well might leave some improvements on the
floor in the importer in order to avoid destroying the value of
carefully-chosen names.  Before applying the verb criterion, we were
finding too many places where the loss of "With" significantly degraded
understandability.  

You're comparing the results of the more aggressive (verb-less) criteria
with those of the verb-considering criteria, so it's not surprising that
you see a few things that could be better.

>>>> - func blendedColor(fraction fraction: CGFloat, of color: NSColor)
>>>> -> NSColor?  + func blendedColorWithFraction(fraction: CGFloat, of
>>>> color: NSColor) -> NSColor?
>>> 
>>> This doesn’t seem like an improvement. “fraction” and “color” seem
>>> very much like parameters to be separated from the name.
>> 
>> Note that this is the intent of the change, however: blendedColor is
>> a noun phrase describing the result, and “withFraction” is a
>> characteristic of the resulting color.
>
> I understand; I just find it a much less compelling example for
> Jordan’s approach than something like the previously mentioned
> “tracks”.
>
> Perhaps the way to differentiate further cases where “with” in the
> name is useful is: methods that you can imagine being called
> “findFoo”, and take some argument as a search condition, but without
> it, only a single noun remains.

Yes, the working guideline I've been considering is: when you're
describing an aspect of an already-existing thing, such as the media
type of a track, you do so in the base name, and don't use a label.

> Looking through the diff, methods like these:
>
> track(trackID trackID: CMPersistentTrackID) -> AVAssetTrack?
> func tracks(mediaCharacteristic mediaCharacteristic: String) ->
> [AVFragmentedAssetTrack]
> class func devices(mediaType mediaType: String!) -> [AnyObject]!
> class func defaultDevice(mediaType mediaType: String!) -> AVCaptureDevice!
> func connection(mediaType mediaType: String!) -> AVCaptureConnection!
> func mutableTrack(compatibleWith track: AVAssetTrack) ->
> AVMutableCompositionTrack?
>
> Bother me the most. But a lot others, I feel like they’re worse off
> for having “with” back in the name

Some are worse off than with the more-aggressive (verb-less) criterion,
but no worse than what we have today, which is important.

>>>> - class func availableColorSpaces(model model: NSColorSpaceModel)
>>>> -> [NSColorSpace] + class func availableColorSpacesWith(model:
>>>> NSColorSpaceModel) -> [NSColorSpace]
>>>>  func indexOfItem(objectValue object: AnyObject) -> Int + func
>>>> indexOfItemWithObjectValue(object: AnyObject) -> Int
>>> 
>>> Same…
>> 
>> With the same answer: both start with noun phrases describing the
>> result, so the “with” is acting more like “having”, indicating that
>> the parameter is describing the characteristics of the result.
>> 
>>> 
>>> 
>>>> - func reviewUnsavedDocuments(alertTitle title: String?,
>>>> cancellable: Bool, delegate: AnyObject?, didReviewAllSelector:
>>>> Selector, contextInfo: UnsafeMutablePointer<Void>) + func
>>>> reviewUnsavedDocumentsWithAlertTitle(title: String?, cancellable:
>>>> Bool, delegate: AnyObject?, didReviewAllSelector: Selector,
>>>> contextInfo: UnsafeMutablePointer<Void>)
>>> 
>>> This definitely seem like a step back, “reviewUnsavedDocuments”
>>> works really well as a name, without the sort of confusion that the
>>> “tracks” mentioned above has.
>> 
>> Somehow, “review” wasn’t in my list of verbs. I’ll fix this.
>
> Another one:
>
> func invalidateLayoutWith(context: NSCollectionViewLayoutInvalidationContext)
> +  func swapWithMark(sender: AnyObject?)
>
>> 
>>>> - class func mouseEvent(type type: NSEventType, location: Point,
>>>> modifierFlags flags: NSEventModifierFlags, timestamp time:
>>>> TimeInterval, windowNumber wNum: Int, context: NSGraphicsContext?,
>>>> eventNumber eNum: Int, clickCount cNum: Int, pressure: Float) ->
>>>> NSEvent?
>>>> + class func mouseEventWith(type: NSEventType, location: Point,
>>>> modifierFlags flags: NSEventModifierFlags, timestamp time:
>>>> TimeInterval, windowNumber wNum: Int, context: NSGraphicsContext?,
>>>> eventNumber eNum: Int, clickCount cNum: Int, pressure: Float) ->
>>>> NSEvent?
>>> 
>>> This one’s weird. “With” was added, but without “type” in the name.
>> 
>> “type” is redundant with the type info, so it has been pruned.
>
> But is “mouseEventWith” as a name actually desirable?

This one might just be an outlier.

> * * *
>
> Another thing that bothers me is how this makes method families have different names:
>
>> - func indexOfItem(title aTitle: String) -> Int + func
>> indexOfItemWithTitle(aTitle: String) -> Int
>> - func indexOfItem(tag aTag: Int) -> Int + func
>> indexOfItemWithTag(aTag: Int) -> Int
>> - func indexOfItem(representedObject object: AnyObject) -> Int +
>> func indexOfItemWithRepresentedObject(object: AnyObject) -> Int
>> - func indexOfItem(submenu submenu: NSMenu?) -> Int + func
>> indexOfItemWithSubmenu(submenu: NSMenu?) -> Int
>> - func indexOfItem(target target: AnyObject?, andAction
>> actionSelector: Selector) -> Int + func
>> indexOfItemWithTarget(target: AnyObject?, andAction actionSelector:
>> Selector) -> Int
>
> I think it would be desirable for this, being essentially a few
> versions of the same method, just taking different arguments, to have
> a single name.

Technically, the text of the argument labels are part of the method
name, but I understand what you mean: it would be really nice to be able
to group these under a single *base* name.  I think this particular
family might be better expressed as something like:

  func indexOfItemWhere(attributes: ViewAttributes) -> Int

where ViewAttributes is a type that can be built with expressions like:

  .submenu == mySubmenu && .tag == someTag

or maybe is just replaced with a closure.  

   indexOf { $0.subMenu == mySubMenu && $0.tag == someTag }

I am generally wary of bending over backward in the guidelines or
importer to accomodate method families, as understanding them creates a
lot of avoidable mental overhead for the programmer.

> This also passes the “you could imagine the name starting with ‘find’” test.
>
> * * *
>
>>   func smartMagnify(event event: NSEvent)		
>> +  func smartMagnifyWith(event: NSEvent)
>>    @available(OSX 10.6, *)		   @available(OSX 10.6, *)
>> -  func touchesBegan(event event: NSEvent)		
>> +  func touchesBeganWith(event: NSEvent)
>>    @available(OSX 10.6, *)		   @available(OSX 10.6, *)
>> -  func touchesMoved(event event: NSEvent)		
>> +  func touchesMovedWith(event: NSEvent)
>>    @available(OSX 10.6, *)		   @available(OSX 10.6, *)
>> -  func touchesEnded(event event: NSEvent)		
>> +  func touchesEndedWith(event: NSEvent)
>>    @available(OSX 10.6, *)		   @available(OSX 10.6, *)
>> -  func touchesCancelled(event event: NSEvent)		
>> +  func touchesCancelledWith(event: NSEvent)
>>    @available(OSX 10.8, *)		   @available(OSX 10.8, *)
>> -  func quickLook(event event: NSEvent)		
>> +  func quickLookWith(event: NSEvent)
>>    @available(OSX 10.10.3, *)		   @available(OSX 10.10.3, *)
>> -  func pressureChange(event event: NSEvent)		
>> +  func pressureChangeWith(event: NSEvent)
>
> Not digging this either. Passing NSEvent here feels like passing
> “sender” — an idiom that seems in no need of adding a “With” suffix to
> the method name…

I suspect we're going to find lots of individual places where we can
afford to do more than the importer will do, but we can't afford to let
the importer go so far in transforming APIs that it creates significant
damage.

-- 
-Dave