<html><head><meta http-equiv="Content-Type" content="text/html charset=utf-8"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class=""><br class=""><div><blockquote type="cite" class=""><div class="">On Jan 27, 2016, at 8:09 AM, Matthew Johnson &lt;<a href="mailto:matthew@anandabits.com" class="">matthew@anandabits.com</a>&gt; wrote:</div><br class="Apple-interchange-newline"><div class=""><meta http-equiv="Content-Type" content="text/html charset=utf-8" class=""><div style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class=""><div class="">Doug,</div><div class=""><br class=""></div><div class="">I think this change looks great! &nbsp;I don’t have time to look through the full patch but did look through quite a bit. &nbsp;It adds clarity in the vast majority of cases I looked at. &nbsp;</div><div class=""><br class=""></div><div class="">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. &nbsp;I agree with the comments earlier on that in these cases a label for the first parameter is the best approach.</div><div class=""><br class=""></div><div class="">I also really like that this groups methods with the same fundamental operation into overload families where they previously had independent names. &nbsp;This is a big win IMO.</div><div class=""><br class=""></div><div class="">There is a first-parameter-is-an-ID pattern I noticed after this change. &nbsp;I show a few examples here, but there are a lot more:</div><div class=""><br class="">- &nbsp;func&nbsp;trackWithTrackID(trackID: CMPersistentTrackID)&nbsp;-&gt;&nbsp;AVAssetTrack?<br class="">+ &nbsp;func&nbsp;track(trackID&nbsp;trackID: CMPersistentTrackID)&nbsp;-&gt;&nbsp;AVAssetTrack?</div><div class=""><br class=""></div><div class="">- &nbsp;func&nbsp;trackWithTrackID(trackID: CMPersistentTrackID)&nbsp;-&gt;&nbsp;AVFragmentedAssetTrack?<br class="">+ &nbsp;func&nbsp;track(trackID&nbsp;trackID: CMPersistentTrackID)&nbsp;-&gt;&nbsp;AVFragmentedAssetTrack?</div><div class=""><br class="">- &nbsp;func&nbsp;trackWithTrackID(trackID: CMPersistentTrackID) -&gt; AVCompositionTrack?<br class="">+ &nbsp;func&nbsp;track(trackID&nbsp;trackID: CMPersistentTrackID) -&gt; AVCompositionTrack?</div><div class=""><br class=""></div><div class="">- &nbsp;func&nbsp;discoverUserInfoWithUserRecordID(userRecordID: CKRecordID, completionHandler: (CKDiscoveredUserInfo?, Error?)&nbsp;-&gt;&nbsp;Void)</div><div class=""><br class="">+ &nbsp;func&nbsp;discoverUserInfo(userRecordID&nbsp;userRecordID: CKRecordID, completionHandler: (CKDiscoveredUserInfo?, Error?)&nbsp;-&gt;&nbsp;Void)</div><div class=""><br class=""></div><div class="">The first argument label `trackID` seems like it repeats type information without adding clarity. &nbsp;I think it would be better to just use `id` here. &nbsp;It seems like a candidate for heuristics as well. &nbsp;For example, if the type name ends in ID and the label is a suffix of the type name we could just use `id`. &nbsp;This is a somewhat specific pattern, but IDs are common enough that it might make sense.</div></div></div></blockquote><div><br class=""></div><div>It affects 33 APIs; see attached patch.</div><div><br class=""></div><blockquote type="cite" class=""><div class=""><div style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class=""><div class="">Interestingly, in at least one case the `WithID` was the original name of the method so we did receive a simple `id` label:</div><div class=""><br class=""></div><div class="">- &nbsp;func&nbsp;parameterWithID(paramID: AudioUnitParameterID, scope: AudioUnitScope, element: AudioUnitElement)&nbsp;-&gt;&nbsp;AUParameter?<br class="">+ &nbsp;func&nbsp;parameter(id&nbsp;paramID: AudioUnitParameterID, scope: AudioUnitScope, element: AudioUnitElement)&nbsp;-&gt;&nbsp;AUParameter?</div></div></div></blockquote><div><br class=""></div><div>What’s happening here is that we see that “ID” in “WithID” is redundant, but we don’t want to provide a first argument label of “with”, so we grab whatever followed “with”.</div><br class=""><blockquote type="cite" class=""><div class=""><div style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class=""><div class="">In another case, the method has a naked `With` at the end. &nbsp;Somehow `id` was used in that scenario despite the parameter name being `objectID` and the type being `NSManagedObjectID`, which aligns with my suggested naming:</div><div class=""><br class=""></div><div class="">- &nbsp;func&nbsp;newValuesForObjectWith(objectID: NSManagedObjectID, withContext&nbsp;context: NSManagedObjectContext) throws&nbsp;-&gt;&nbsp;NSIncrementalStoreNode<br class="">+ &nbsp;func&nbsp;newValuesForObject(id&nbsp;objectID: NSManagedObjectID, withContext&nbsp;context: NSManagedObjectContext) throws&nbsp;-&gt;&nbsp;NSIncrementalStoreNode</div></div></div></blockquote><div><br class=""></div><div>This was originally newValuesForObjectWithID; again, we trimmed ID both before and after, and you’re seeing us keeping “id” because it’s better than keeping “with”.</div><br class=""><blockquote type="cite" class=""><div class=""><div style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class=""><div class=""><br class=""></div><div class="">A case related to that used `iDs` for array arguments: &nbsp;</div><div class=""><br class=""></div><div class="">- &nbsp;func&nbsp;managedObjectContextDidRegisterObjectsWithIDs(objectIDs: [NSManagedObjectID])<br class="">- &nbsp;func&nbsp;managedObjectContextDidUnregisterObjectsWithIDs(objectIDs: [NSManagedObjectID])<br class="">+ &nbsp;func&nbsp;managedObjectContextDidRegisterObjects(iDs&nbsp;objectIDs: [NSManagedObjectID])<br class="">+ &nbsp;func&nbsp;managedObjectContextDidUnregisterObjects(iDs&nbsp;objectIDs: [NSManagedObjectID])</div><div class=""><br class=""></div><div class="">I would prefer `ids` here. &nbsp;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.</div></div></div></blockquote><div><br class=""></div><div>Ah, it looks like I'm handling lowercasing of plural initialisms badly. That’s a problem independent of first argument labels; thanks!</div><br class=""><blockquote type="cite" class=""><div class=""><div style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class=""><div class=""><br class=""></div><div class=""><br class=""></div><div class="">Here’s another interesting change:</div><div class=""><br class="">- &nbsp;func&nbsp;unionWith(s2: CIFilterShape)&nbsp;-&gt;&nbsp;CIFilterShape<br class="">- &nbsp;func&nbsp;unionWith(r: CGRect)&nbsp;-&gt;&nbsp;CIFilterShape<br class="">- &nbsp;func&nbsp;intersectWith(s2: CIFilterShape)&nbsp;-&gt;&nbsp;CIFilterShape<br class="">- &nbsp;func&nbsp;intersectWith(r: CGRect)&nbsp;-&gt;&nbsp;CIFilterShape<br class="">+ &nbsp;func&nbsp;union(with&nbsp;s2: CIFilterShape)&nbsp;-&gt;&nbsp;CIFilterShape<br class="">+ &nbsp;func&nbsp;union(rect&nbsp;r: CGRect)&nbsp;-&gt;&nbsp;CIFilterShape<br class="">+ &nbsp;func&nbsp;intersect(with&nbsp;s2: CIFilterShape)&nbsp;-&gt;&nbsp;CIFilterShape<br class="">+ &nbsp;func&nbsp;intersect(rect&nbsp;r: CGRect)&nbsp;-&gt;&nbsp;CIFilterShape</div><div class=""><br class=""></div><div class="">Why do the CGRect arguments receive a type-derived label but the CIFilterShape arguments just receive `with`? &nbsp;Shouldn’t these follow the same pattern?</div></div></div></blockquote><div><br class=""></div>The Objective-C methods are actually named unionWith: and unionWithRect:. That first name is not following Cocoa conventions.<br class=""><div><br class=""></div><span class="Apple-tab-span" style="white-space:pre">        </span>- Doug</div><div><br class=""></div><div></div></body></html>