> On Apr 5, 2017, at 5:45 PM, Ben Cohen via swift-evolution 
> <[email protected]> wrote:
> 
>       • What is your evaluation of the proposal?

(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.)

Sequence-based initializer and merging initializer

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:

        init<S: Sequence>(_ keysAndValues: S, correctingConflictsBy 
resolveConflict: (Value, Value) throws -> Value = { fatalError("Duplicate 
values \($0) and \($1)") }) rethrows
                where S.Iterator.Element == (key: Key, value: Value)

Merging methods

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.

I wonder if we might also want a method that copies the Dictionary, but with a 
single key added/removed/changed:

        func withValue(_ value: Value?, forKey key: Key) -> [Key: Value]

Key-based subscript with default value

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 `??`:

        frequencies[c] ?? 0 += 1

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.

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.

Dictionary-specific map and filter

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: 
<http://stackoverflow.com/questions/24116271/whats-the-cleanest-way-of-applying-map-to-a-dictionary-in-swift/24219069#24219069>

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.

Visible dictionary capacity

I doubt I'll ever use the `capacity` property, but I'm not opposed to it. 
Adding a `reserveCapacity(_:)` method is a good idea.

A grouped(by:) method for sequences

Yes, please.

Apply relevant changes to Set

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.)

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.

General notes

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.

>       • Is the problem being addressed significant enough to warrant a change 
> to Swift?

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.

>       • Does this proposal fit well with the feel and direction of Swift?

Yes, these are all pretty straightforward additions.

>       • If you have used other languages or libraries with a similar feature, 
> how do you feel that this proposal compares to those?

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.

>       • How much effort did you put into your review? A glance, a quick 
> reading, or an in-depth study?


Quick reading.

-- 
Brent Royal-Gordon
Architechies

_______________________________________________
swift-evolution mailing list
[email protected]
https://lists.swift.org/mailman/listinfo/swift-evolution

Reply via email to