Skip to main content
edited body
Source Link
Martin R
  • 23.5k
  • 2
  • 36
  • 92

Your code looks generally good (with a some exceptionstwo real problems explained below).

Your code looks generally good (with a some exceptions explained below).

Your code looks generally good (with two real problems explained below).

added 93 characters in body
Source Link
Martin R
  • 23.5k
  • 2
  • 36
  • 92

You either have to add an explicit check (and throw an error or fail if duplicate keys are given), or add the key/value pairs sequentially so that duplicate keys are silently overwritten with the new value:

init(_ tupleArgs: [KeyValueType]) {
    self.init()
    for (key, value) in tupleArgs {
        self[key] = value
    }
}
extension OrderedDictionary : CustomStringConvertible {
    var description: String {
        return "[ "
            + tuples.map({ (key, value) in "\(key) : \(value)"}).joinWithSeparator(", ")
            + "]"
    }
}

Creating a new (ordered) dictionary by mapping both keys and values can lead to unexpected results if two keys are mapped to the same new key. E.g. (with the fixed init method from above):

let od1 = OrderedDictionary(("foo", 1), ("bar", 2), ("FOO", 3))
print(od1) // [ foo : 1, bar : 2, FOO : 3]

let od2 = od1.map { (key, value) in (key.uppercaseString, value) }
print(od2) // [ FOO : 3, BAR : 2]

You have to decide if that is wanted. Alternatively, define a map() method which only maps the values but preserves the keys.

extension OrderedDictionary : CustomStringConvertible {
    var description: String {
        return "[ "
            + tuples.map({ (key, value) in "\(key) : \(value)"}).joinWithSeparator(", ")
            + "]"
    }
}

You either have to add an explicit check (and throw an error or fail if duplicate keys are given), or add the key/value pairs sequentially so that duplicate keys are silently overwritten with the new value:

init(_ tupleArgs: [KeyValueType]) {
    self.init()
    for (key, value) in tupleArgs {
        self[key] = value
    }
}
extension OrderedDictionary : CustomStringConvertible {
    var description: String {
        return "[ "
            + tuples.map({ (key, value) in "\(key) : \(value)"}).joinWithSeparator(", ")
            + "]"
    }
}

Creating a new (ordered) dictionary by mapping both keys and values can lead to unexpected results if two keys are mapped to the same new key. E.g. (with the fixed init method from above):

let od1 = OrderedDictionary(("foo", 1), ("bar", 2), ("FOO", 3))
print(od1) // [ foo : 1, bar : 2, FOO : 3]

let od2 = od1.map { (key, value) in (key.uppercaseString, value) }
print(od2) // [ FOO : 3, BAR : 2]

You have to decide if that is wanted. Alternatively, define a map() method which only maps the values but preserves the keys.

added 93 characters in body
Source Link
Martin R
  • 23.5k
  • 2
  • 36
  • 92

Your code looks generally good (with a single exceptionsome exceptions explained below).

but that is perhaps a matter of taste. 

The problem with the other two init methods are OK, in particularis that init(_ tupleArgs: KeyValueType...) forwards tothey don't check init(_ tupleArgs: [KeyValueType])for duplicate keys, e.g. if you call

let od = OrderedDictionary(("foo", 1), ("bar", 2), ("foo", 3))
extension OrderedDictionary : CustomStringConvertible {
    
    var description: String {
        return toDict"[ "
            + tuples.map({ (key, value) in "\(key) : \(value)"}).descriptionjoinWithSeparator(", ")
            + "]"
    }
}

Your code looks generally good (with a single exception explained below).

but that is perhaps a matter of taste. The other two init methods are OK, in particular that init(_ tupleArgs: KeyValueType...) forwards to init(_ tupleArgs: [KeyValueType]).

extension OrderedDictionary : CustomStringConvertible {
    
    var description: String {
        return toDict().description
    }
}

Your code looks generally good (with a some exceptions explained below).

but that is perhaps a matter of taste. 

The problem with the other two init methods is that they don't check for duplicate keys, e.g. if you call

let od = OrderedDictionary(("foo", 1), ("bar", 2), ("foo", 3))
extension OrderedDictionary : CustomStringConvertible {
    var description: String {
        return "[ "
            + tuples.map({ (key, value) in "\(key) : \(value)"}).joinWithSeparator(", ")
            + "]"
    }
}
Source Link
Martin R
  • 23.5k
  • 2
  • 36
  • 92
Loading