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

Matthew Johnson matthew at anandabits.com
Wed Jan 27 12:59:20 CST 2016 

> On Jan 27, 2016, at 12:03 PM, Dave Abrahams via swift-evolution <swift-evolution at swift.org> wrote:
> 
> 
> on Wed Jan 27 2016, Matthew Johnson <swift-evolution at swift.org <mailto:swift-evolution at swift.org>> wrote:
> 
>> Doug,
>> 
>> I think this change looks great!  I don’t have time to look through
>> the full patch but did look through quite a bit.  It adds clarity in
>> the vast majority of cases I looked at.
>> 
>> It seems like with-as-separator is a good heuristic for determining
>> when the first parameter is not essential to a good name for the
>> fundamental operation.  I agree with the comments earlier on that in
>> these cases a label for the first parameter is the best approach.
>> 
>> I also really like that this groups methods with the same fundamental
>> operation into overload families where they previously had independent
>> names.  This is a big win IMO.
>> 
>> There is a first-parameter-is-an-ID pattern I noticed after this
>> change.  I show a few examples here, but there are a lot more:
>> 
>> -  func trackWithTrackID(trackID: CMPersistentTrackID) -> AVAssetTrack?
>> +  func track(trackID trackID: CMPersistentTrackID) -> AVAssetTrack?
>> 
>> -  func trackWithTrackID(trackID: CMPersistentTrackID) -> AVFragmentedAssetTrack?
>> +  func track(trackID trackID: CMPersistentTrackID) -> AVFragmentedAssetTrack?
>> 
>> -  func trackWithTrackID(trackID: CMPersistentTrackID) -> AVCompositionTrack?
>> +  func track(trackID trackID: CMPersistentTrackID) -> AVCompositionTrack?
>> 
>> - func discoverUserInfoWithUserRecordID(userRecordID: CKRecordID,
>> completionHandler: (CKDiscoveredUserInfo?, Error?) -> Void)
>> 
>> + func discoverUserInfo(userRecordID userRecordID: CKRecordID,
>> completionHandler: (CKDiscoveredUserInfo?, Error?) -> Void)
>> 
>> The first argument label `trackID` seems like it repeats type
>> information without adding clarity.  I think it would be better to
>> just use `id` here.  It seems like a candidate for heuristics as well.
>> For example, if the type name ends in ID and the label is a suffix of
>> the type name we could just use `id`.  This is a somewhat specific
>> pattern, but IDs are common enough that it might make sense.
> 
> Actually I've been saying for a while that arguments called ID,
> identifier, and name should not be labelled at all in many cases.  Think
> about it.

Looking at this closer, I agree in the single argument case.  However, in cases where there are other closely related arguments I think it feels odd to not label the id parameter.  Regardless, I think the ID case deserves special consideration if we go with Doug’s strategy of using `With` as a separator.

> 
>> Interestingly, in at least one case the `WithID` was the original name
>> of the method so we did receive a simple `id` label:
>> 
>> - func parameterWithID(paramID: AudioUnitParameterID, scope:
>> AudioUnitScope, element: AudioUnitElement) -> AUParameter?
>> + func parameter(id paramID: AudioUnitParameterID, scope:
>> AudioUnitScope, element: AudioUnitElement) -> AUParameter?
>> 
>> In another case, the method has a naked `With` at the end.  Somehow
>> `id` was used in that scenario despite the parameter name being
>> `objectID` and the type being `NSManagedObjectID`, which aligns with
>> my suggested naming:
>> 
>> - func newValuesForObjectWith(objectID: NSManagedObjectID, withContext
>> context: NSManagedObjectContext) throws -> NSIncrementalStoreNode
>> + func newValuesForObject(id objectID: NSManagedObjectID, withContext
>> context: NSManagedObjectContext) throws -> NSIncrementalStoreNode
>> 
>> A case related to that used `iDs` for array arguments:  
>> 
>> -  func managedObjectContextDidRegisterObjectsWithIDs(objectIDs: [NSManagedObjectID])
>> -  func managedObjectContextDidUnregisterObjectsWithIDs(objectIDs: [NSManagedObjectID])
>> +  func managedObjectContextDidRegisterObjects(iDs objectIDs: [NSManagedObjectID])
>> +  func managedObjectContextDidUnregisterObjects(iDs objectIDs: [NSManagedObjectID])
>> 
>> I would prefer `ids` here.  This seems like a pattern that would be a
>> problem for any all-caps plural acronym or initialism so it might be
>> good to add a heuristic for this as well.
>> 
>> Here’s another interesting change:
>> 
>> -  func unionWith(s2: CIFilterShape) -> CIFilterShape
>> -  func unionWith(r: CGRect) -> CIFilterShape
>> -  func intersectWith(s2: CIFilterShape) -> CIFilterShape
>> -  func intersectWith(r: CGRect) -> CIFilterShape
>> +  func union(with s2: CIFilterShape) -> CIFilterShape
>> +  func union(rect r: CGRect) -> CIFilterShape
>> +  func intersect(with s2: CIFilterShape) -> CIFilterShape
>> +  func intersect(rect r: CGRect) -> CIFilterShape
>> 
>> Why do the CGRect arguments receive a type-derived label but the
>> CIFilterShape arguments just receive `with`?  Shouldn’t these follow
>> the same pattern?
>> 
>> -Matthew
>> 
>>> On Jan 27, 2016, at 1:50 AM, Douglas Gregor via swift-evolution
>>> <swift-evolution at swift.org> wrote:
>>> 
>>> 
>>>> On Jan 25, 2016, at 6:55 AM, Radosław Pietruszewski
>>>> <radexpl at gmail.com
>>>> <mailto:radexpl at gmail.com <mailto:radexpl at gmail.com>>> wrote:
>>>> 
>>>> Hello all,
>>>> 
>>>> I’m overwhelmingly *for* this proposal. I think removing needless
>>>> verbosity and keeping the signal-to-noise ratio high is one of the
>>>> most immediately appealing aspects of Swift, as well as a great
>>>> general improvement to the programming experience.
>>>> 
>>>> And so unswiftified (yes, it’s a word now) APIs from Objective-C
>>>> stick out like a sore thumb. Not only are they harder to read and
>>>> write, they visually overwhelm the less verbose, information-dense
>>>> Swift-first code.
>>>> 
>>>> Just like previous 1.0—2.0 attempts at bridging the gap (with
>>>> NSError params being translated to Swift errors, factory methods
>>>> translated to initializers, etc.), automating this will be an
>>>> error-prone process, and almost bound to be a bit annoying at
>>>> first, before all the glitches and poor translations are smoothed
>>>> out. And yet I feel like just like the previous automated
>>>> translations were overwhelmingly a great thing, so will the result
>>>> of this proposal.
>>>> 
>>>> * * *
>>>> 
>>>>>   Add First Argument Labels
>>>>> 
>>>>>   - func enumerateObjectsWith(_: NSEnumerationOptions = [],
>>>>> using: (AnyObject, UnsafeMutablePointer) -> Void)
>>>>>   + func enumerateObjects(options _: NSEnumerationOptions = [],
>>>>> using: (AnyObject, UnsafeMutablePointer) -> Void)
>>>> 
>>>> Good! The Guidelines recommend an explicit first parameter label
>>>> for arguments with a default value, but this is a good change also
>>>> for another reason, a use case not included in the Guidelines (I
>>>> have more to say about this in the SE-0023 thread):
>>>> 
>>>> “Options” is the description of the parameter, not the method
>>>> itself. Even if (for whatever reason!) `options` didn’t have a
>>>> default value and the word “Options” wasn’t omitted in the
>>>> translation,
>>>> 
>>>>   enumerateObjects(options: …)
>>>> 
>>>> would be clearer than
>>>> 
>>>>   enumerateObjectsWithOptions(…)
>>>> 
>>>> It’s not even about the extra word, about the four useless
>>>> characters, it’s simply that “WithOptions” doesn’t describe the
>>>> operation at all. It’s a word that conveys no information (“with”),
>>>> and “options”, which describes the first parameter. In Objective-C,
>>>> there’s no such thing as parameter labels, it’s all one name, so
>>>> “With” is used as a separator. But in Swift, making the first
>>>> parameter’s label explicit just makes more sense.
>>> 
>>> That’s an interesting thought! If “with” is truly used as a
>>> convention for separating the description of the operation from the
>>> description of the first parameter, that’s something that can be
>>> codified in the Clang importer. I was curious, so I hacked it
>>> up. Here’s a diff of the Cocoa APIs that shows what things would
>>> look like if we treated “with” as a separator:
>>> 
>>> 	https://github.com/apple/swift-3-api-guidelines-review/pull/5/files <https://github.com/apple/swift-3-api-guidelines-review/pull/5/files>
>>> <https://github.com/apple/swift-3-api-guidelines-review/pull/5/files <https://github.com/apple/swift-3-api-guidelines-review/pull/5/files>>
>>> 
>>> It’s a diff against SE-0005, and it introduces a significant number
>>> of first argument labels. Indeed, you’ll need to grab the patch to
>>> see them all:
>>> 
>>> 	https://github.com/apple/swift-3-api-guidelines-review/pull/5.patch <https://github.com/apple/swift-3-api-guidelines-review/pull/5.patch>
>>> <https://github.com/apple/swift-3-api-guidelines-review/pull/5.patch <https://github.com/apple/swift-3-api-guidelines-review/pull/5.patch>>
>>> 
>>> A brief survey shows that some cases seem to be lining up with the
>>> guideline proposals that have been under discussion. For example,
>>> the patch includes:
>>> 
>>> -  func fillWith(blendMode: CGBlendMode, alpha: CGFloat)
>>> -  func strokeWith(blendMode: CGBlendMode, alpha: CGFloat)
>>> +  func fill(blendMode blendMode: CGBlendMode, alpha: CGFloat)
>>> +  func stroke(blendMode blendMode: CGBlendMode, alpha: CGFloat)
>>> 
>>> -  func encodeWith(aCoder: Coder)
>>> +  func encode(coder aCoder: Coder)
>>> 
>>> which you might recognize, because it’s the example you used:
>>> 
>>>> And with that in mind, I object to these translations:
>>>> 
>>>>   func fillWith(_: CGBlendMode, alpha: CGFloat)
>>>>   func strokeWith(_: CGBlendMode, alpha: CGFloat)
>>>>   func encodeWith(_: Coder)
>>>> 
>>>> Even though these don’t have default values, I believe this version
>>>> to be clearer and make more sense, even if slightly more verbose:
>>>> 
>>>>   func fill(blendMode: CGBlendMode, alpha: CGFloat)
>>>>   func stroke(blendMode: CGBlendMode, alpha: CGFloat)
>>>>   func encode(coder: Coder)
>>> 
>>> Another random interesting example I encountered:
>>> 
>>> -  func addArcWithCenter(center: CGPoint, radius: CGFloat, startAngle: CGFloat, endAngle: CGFloat, clockwise: Bool)
>>> +  func addArc(center center: CGPoint, radius: CGFloat, startAngle: CGFloat, endAngle: CGFloat, clockwise: Bool)
>>> 
>>> which seems to match the idea behind Erica’s "semantic relationship between the parameters is stronger than their relation to the operation” (or Paul Cantrell’s similar notion of "the direct object is several args taken together”, which feels more in line with the way the API guidelines are written).
>>> 
>>> There’s also this:
>>> 
>>> -  func tracksWithMediaType(mediaType: String) -> [AVMovieTrack]
>>> +  func tracks(mediaType mediaType: String) -> [AVMovieTrack]
>>> 
>>> -  func tracksWithMediaCharacteristic(mediaCharacteristic: String) ->
>>> +  func tracks(mediaCharacteristic mediaCharacteristic: String) -> [AVMovieTrack]
>>> 
>>> which feels reminiscent of Paul’s “resource” example:
>>> 
>>>    service.resource("/foo")
>>>    service.resource(absoluteURL: "http://bar.com <http://bar.com/> <http://bar.com/ <http://bar.com/>>")
>>>    service.resource(absoluteURL: NSURL(string: "http://bar.com <http://bar.com/> <http://bar.com/ <http://bar.com/>>"))
>>> 
>>> where (I think) the argument is that the various methods should all have the same base name because they’re all returning “tracks” or a “resource”, respectively.
>>> 
>>> There is a ton of data in that patch. I’d be interested to hear whether the resulting Cocoa APIs feel better in Swift—are they following the evolving set of guidelines for first argument labels that are under discussion, and are the resulting APIs clearer/more Swifty? What specific APIs work well and where does this “with-as-separator” heuristic break down?
>>> 
>>> 	- Doug
>>> 
>>> 
>>> _______________________________________________
>>> 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>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.swift.org/pipermail/swift-evolution/attachments/20160127/2971cad0/attachment.html>

More information about the swift-evolution mailing list