<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=""><div><blockquote type="cite" class=""><div class="">On Apr 5, 2017, at 5:45 PM, Ben Cohen via swift-evolution <<a href="mailto:swift-evolution@swift.org" class="">swift-evolution@swift.org</a>> wrote:</div><div class=""><div style="word-wrap: break-word; -webkit-nbsp-mode: space; line-break: after-white-space;" class=""><div class=""><div class=""><br class=""><div class=""><span class="Apple-tab-span" style="white-space: pre;"> </span>• What is your evaluation of the proposal?<br class=""></div></div></div></div></div></blockquote><div><br class=""></div><div>(As a meta issue, I'm not sure I like the grab-bag review style; I'm finding this proposal a little bit difficult to navigate.)</div><div><br class=""></div><div><b class="">Sequence-based initializer and merging initializer</b></div><div><br class=""></div><div>Good idea, but I think these two are redundant with each other, and I don't think "merging" is an accurate way to describe what the second one does. (`merging` would suggest to me that it was combining several dictionaries or lists, not combining conflicting elements.) I'd suggest a single initializer along the lines of:</div><div><br class=""></div><div><span class="Apple-tab-span" style="white-space:pre"> </span>init<S: Sequence>(_ keysAndValues: S, correctingConflictsBy resolveConflict: (Value, Value) throws -> Value = { fatalError("Duplicate values \($0) and \($1)") }) rethrows</div><div><span class="Apple-tab-span" style="white-space:pre"> </span>where S.Iterator.Element == (key: Key, value: Value)</div><div><br class=""></div><div><b class="">Merging methods</b></div><div><br class=""></div><div>Good idea, but I'm not a fan of the `mergingValues:` label. I would suggest the same `correctingConflictsBy resolveConflict:` label I suggested for the previous method—possibly including the default value. I also think `merge(_:correctingConflictsBy:)`'s first parameter should be labeled `with`, just as the `merged` variant is.</div><div><br class=""></div><div>I wonder if we might also want a method that copies the Dictionary, but with a single key added/removed/changed:</div><div><br class=""></div><div><span class="Apple-tab-span" style="white-space:pre"> </span>func withValue(_ value: Value?, forKey key: Key) -> [Key: Value]</div><div><br class=""></div><div><b class="">Key-based subscript with default value</b></div><div><br class=""></div><div>I like the functionality, but not way this proposal does it. I don't like having the default value be a labeled parameter to the subscript, because it isn't used to locate the value. However, I can't come up with a better syntax without adding language features. What I'd like to do is make it possible to assign through `??`:</div><div><br class=""></div><div><span class="Apple-tab-span" style="white-space:pre"> </span>frequencies[c] ?? 0 += 1</div><div><br class=""></div><div>But that would require either that we support `inout` functions, or that `??` become magic syntax instead of a standard library feature. The former is not coming in Swift 4 and the latter is less than ideal.</div><div><br class=""></div><div>Still, if we would rather have that syntax and we think we'll soon have the language improvements needed to pull it off, I'd suggest rejecting this portion of the proposal.</div><div><br class=""></div><div><b class="">Dictionary-specific map and filter</b></div><div><br class=""></div><div>I am +114 on this. I say that because I have received 114 upvotes on my Stack Overflow answer explaining how to write a `Dictionary.map` method: <<a href="http://stackoverflow.com/questions/24116271/whats-the-cleanest-way-of-applying-map-to-a-dictionary-in-swift/24219069#24219069" class="">http://stackoverflow.com/questions/24116271/whats-the-cleanest-way-of-applying-map-to-a-dictionary-in-swift/24219069#24219069</a>></div><div><br class=""></div><div>I agree with the decision not to pass keys to the closures in these methods; that keeps them simple and focused, and ensures they stay parallel with ordinary `map` and `filter`. I also agree with the decision to not build in a form of `map` which allows key remapping; you can always do that with the sequence-based initializer, which would let you add conflict-handling logic more elegantly than a key-value `map` could.</div><div><br class=""></div><div><b class="">Visible dictionary capacity</b></div><div><br class=""></div><div>I doubt I'll ever use the `capacity` property, but I'm not opposed to it. Adding a `reserveCapacity(_:)` method is a good idea.</div><div><br class=""></div><div><b class="">A grouped(by:) method for sequences</b></div><div><br class=""></div><div>Yes, please.</div><div><br class=""></div><div><b class="">Apply relevant changes to Set</b></div><div><br class=""></div><div>These make sense. (I considered suggesting the `filter` method be called `intersection(where:)`, but on second thought, I don't think that conveys the actual use cases for the method very well.)</div><div><br class=""></div><div>I wonder if we might want to conform `Dictionary` to `SetAlgebra`. They have compatible semantics, and I've occasionally wanted to use them in the same places. On the other hand, some of the methods might form attractive nuisances; perhaps I should just write a SetAlgebra-conforming view when I want to do that.</div><div><br class=""></div><div><b class="">General notes</b></div><div><b class=""><br class=""></b></div><div>If SE-0161 is accepted, we may want to support key path variants of some of these APIs (like `grouped(by:)`, `map(_:)`, and `filter(_:)`). On the other hand, we may want to defer that so we can consider that issue holistically, including with existing Collection APIs.</div><div><br class=""></div><blockquote type="cite" class=""><div class=""><div style="word-wrap: break-word; -webkit-nbsp-mode: space; line-break: after-white-space;" class=""><div class=""><div class=""><div class=""><span class="Apple-tab-span" style="white-space: pre;"> </span>• Is the problem being addressed significant enough to warrant a change to Swift?<br class=""></div></div></div></div></div></blockquote><div><br class=""></div><div>Yes. These are all common needs; when working with dictionaries, I find myself writing `for` loops with `var`s far more often than I'd like.</div><br class=""><blockquote type="cite" class=""><div class=""><div style="word-wrap: break-word; -webkit-nbsp-mode: space; line-break: after-white-space;" class=""><div class=""><div class=""><div class=""><span class="Apple-tab-span" style="white-space: pre;"> </span>• Does this proposal fit well with the feel and direction of Swift?<br class=""></div></div></div></div></div></blockquote><div><br class=""></div><div>Yes, these are all pretty straightforward additions.</div><br class=""><blockquote type="cite" class=""><div class=""><div style="word-wrap: break-word; -webkit-nbsp-mode: space; line-break: after-white-space;" class=""><div class=""><div class=""><div class=""><span class="Apple-tab-span" style="white-space: pre;"> </span>• If you have used other languages or libraries with a similar feature, how do you feel that this proposal compares to those?<br class=""></div></div></div></div></div></blockquote><div><br class=""></div><div>Ruby's `Hash` has many of these features and I appreciate them there; `NSDictionary` does not and it suffers for it. Perl hashes have a flattening behavior that tends to be amenable to editing them in various ways, but that's not really suitable for Swift.</div><br class=""><blockquote type="cite" class=""><div class=""><div style="word-wrap: break-word; -webkit-nbsp-mode: space; line-break: after-white-space;" class=""><div class=""><div class=""><div class=""><span class="Apple-tab-span" style="white-space: pre;"> </span>• How much effort did you put into your review? A glance, a quick reading, or an in-depth study?<br class=""></div></div></div></div></div></blockquote></div><div class=""><br class=""></div><div class="">Quick reading.</div><br class=""><div class="">
<span class="Apple-style-span" style="border-collapse: separate; font-variant-ligatures: normal; font-variant-east-asian: normal; font-variant-position: normal; line-height: normal; border-spacing: 0px;"><div class=""><div style="font-size: 12px; " class="">-- </div><div style="font-size: 12px; " class="">Brent Royal-Gordon</div><div style="font-size: 12px; " class="">Architechies</div></div></span>
</div>
<br class=""></body></html>