<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 <<a href="mailto:matthew@anandabits.com" class="">matthew@anandabits.com</a>> 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! 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. </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. 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. 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. I show a few examples here, but there are a lot more:</div><div class=""><br class="">- func trackWithTrackID(trackID: CMPersistentTrackID) -> AVAssetTrack?<br class="">+ func track(trackID trackID: CMPersistentTrackID) -> AVAssetTrack?</div><div class=""><br class=""></div><div class="">- func trackWithTrackID(trackID: CMPersistentTrackID) -> AVFragmentedAssetTrack?<br class="">+ func track(trackID trackID: CMPersistentTrackID) -> AVFragmentedAssetTrack?</div><div class=""><br class="">- func trackWithTrackID(trackID: CMPersistentTrackID) -> AVCompositionTrack?<br class="">+ func track(trackID trackID: CMPersistentTrackID) -> AVCompositionTrack?</div><div class=""><br class=""></div><div class="">- func discoverUserInfoWithUserRecordID(userRecordID: CKRecordID, completionHandler: (CKDiscoveredUserInfo?, Error?) -> Void)</div><div class=""><br class="">+ func discoverUserInfo(userRecordID userRecordID: CKRecordID, completionHandler: (CKDiscoveredUserInfo?, Error?) -> Void)</div><div class=""><br class=""></div><div class="">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.</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="">- func parameterWithID(paramID: AudioUnitParameterID, scope: AudioUnitScope, element: AudioUnitElement) -> AUParameter?<br class="">+ func parameter(id paramID: AudioUnitParameterID, scope: AudioUnitScope, element: AudioUnitElement) -> 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. 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="">- func newValuesForObjectWith(objectID: NSManagedObjectID, withContext context: NSManagedObjectContext) throws -> NSIncrementalStoreNode<br class="">+ func newValuesForObject(id objectID: NSManagedObjectID, withContext context: NSManagedObjectContext) throws -> 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: </div><div class=""><br class=""></div><div class="">- func managedObjectContextDidRegisterObjectsWithIDs(objectIDs: [NSManagedObjectID])<br class="">- func managedObjectContextDidUnregisterObjectsWithIDs(objectIDs: [NSManagedObjectID])<br class="">+ func managedObjectContextDidRegisterObjects(iDs objectIDs: [NSManagedObjectID])<br class="">+ func managedObjectContextDidUnregisterObjects(iDs objectIDs: [NSManagedObjectID])</div><div class=""><br class=""></div><div class="">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.</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="">- func unionWith(s2: CIFilterShape) -> CIFilterShape<br class="">- func unionWith(r: CGRect) -> CIFilterShape<br class="">- func intersectWith(s2: CIFilterShape) -> CIFilterShape<br class="">- func intersectWith(r: CGRect) -> CIFilterShape<br class="">+ func union(with s2: CIFilterShape) -> CIFilterShape<br class="">+ func union(rect r: CGRect) -> CIFilterShape<br class="">+ func intersect(with s2: CIFilterShape) -> CIFilterShape<br class="">+ func intersect(rect r: CGRect) -> 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`? 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>