Add sequence-based initializers and merge methods to Dictionary#125
Conversation
There was a problem hiding this comment.
"Cumbersome and unfriendly" is a subjective judgement, unfortunately. One can also say that checking for key uniqueness is "preventing errors" and "is making code more explicit". I think both points are valid, but we need more data to understand how much convenience we are getting with implicit behavior that drops the keys vs. how many bugs we would be hiding that way. Please keep in mind that these could easily be security bugs, if one, say, uses dictionary to handle HTTP headers.
There was a problem hiding this comment.
That's a good point. One solution could be to make the initializer/methods without the combine closure parameter failable or marked throws. A caller would then have to reckon with the possibility of duplicate keys one of three ways:
- Let the initializer check for duplicate keys and accept a
nilreturn if duplicates exist, - Unwrap the resulting optional dictionary with
!—this could be used if they check first or have static knowledge that the sequence doesn't have any duplicate keys, or - Provide a closure to handle any cases with duplicate keys.
Someone wanting a Dictionary back after filtering a dictionary on the value could use the second option, since the intermediate sequence would have a subset of the original dictionary's keys. I'm not sure of a case where someone would want the first option, which would likely push people toward the other two options.
Another solution could be to always require the closure, and in the documentation provide { (old, _) in old } and { (_, new) in new } as possible examples for keeping the first and last values, respectively.
Interestingly enough, NSDictionary opts for the exact opposite behavior as this proposal, silently keeping the first value given for a key when created via dictionaryWithObjects:forKeys::
NSDictionary(objects: [1, 2, 3, 4], forKeys: ["a", "b", "c", "a"])
// ["b": 2, "c": 3, "a": 1]
While NSMutableDictionary aligns with this proposal, silently keeping the last value!
NSMutableDictionary(objects: [1, 2, 3, 4], forKeys: ["a", "b", "c", "a"])
// ["b": 2, "c": 3, "a": 4]
So, I suppose it would be nice to have something consistent for Dictionary in Swift if this proposal is accepted.
There was a problem hiding this comment.
I also want to note that having the ! in users code helps a lot with pointing out the possible trap.
There was a problem hiding this comment.
FWIW this is what I've been using so far:
extension Dictionary {
/**
Creates a dictionary from an array of key-value pairs.
Will assert if a key is repeated and !ignoringDuplicateKeys.
*/
public init(elements: [(Key, Value)], ignoringDuplicateKeys: Bool = false) {
I agree that not ignoring duplicates by default is probably the safest. The problem with returning nil in the case of found duplicates is that there would be no way to ignore them other than ensuring that the input is free of duplicates.
There was a problem hiding this comment.
I'm modifying the proposal to convert the simple init to be a failable init? and the merging init to use a marging label for the sequence: init(merging:combine:). The new APIs under the revised proposal would be:
init?<S: SequenceType where S.Generator.Element == (Key, Value)>(
_ sequence: S)
init<S: SequenceType where S.Generator.Element == (Key, Value)>(
merging sequence: S,
@noescape combine: (Value, Value) throws -> Value
) rethrows
mutating func mergeContentsOf<S: SequenceType where S.Generator.Element == (Key, Value)>(
_ sequence: S,
@noescape combine: (Value, Value) throws -> Value
) rethrows
func mergedWith<S: SequenceType where S.Generator.Element == (Key, Value)>(
_ sequence: S,
@noescape combine: (Value, Value) throws -> Value) rethrows -> [Key: Value]@gribozavr: I think this should address your concerns and let the caller decide what level of error checking they want. Thanks again for your comments, they really helped me clarify the difference between the two kinds of functionality requested here.
@NachoSoto: With this revision you would use the merging init to ignore duplicate keys and choose specifically which value to take. How does this look?
let dupes = [("a", 1), ("b", 2), ("a", 3), ("b", 4)]
let firstValueDictionary = Dictionary(merging: dupes) { (first, _) in first }
// ["b": 2, "a": 1]
let lastValueDictionary = Dictionary(merging: dupes) { (_, last) in last }
// ["b": 4, "a": 3]8af5ddb to
b28bd4c
Compare
|
@dabrahams Updated for Swift 3 APIs—any other info I can provide on this proposal? |
|
@dabrahams What's the status on this, does this make sense to merge? If so, please do and link it into schedule.md as an unscheduled proposal. Thanks. |
The
Dictionarytype should allow initialization from a sequence of(Key, Value)tuples and offer methods that merge a sequence of(Key, Value)tuples with an existing dictionary. Each of these new APIs would optionally take a closure to combine values for duplicate keys.View Proposal →