Sean Cook Sean Cook - 4 months ago 25
Swift Question

Dictionary reference not updating dictionary

I have a reference to a dictionary and when I debug the code I can see that the reference is being populated but the actual dictionary is not getting any of the values.

var newItems = [String:Dictionary<String,AnyObject>]()

for item in self.items{
let itemRef = ref.child("items").childByAutoId()
//let itemOptions: NSMutableArray = []
//let itemRemovedOptions: NSMutableArray = []
//let itemDiscounts: NSMutableArray = []

newItems.updateValue([String:AnyObject](), forKey: "\(itemRef.key)")

newItems["\(itemRef.key)"]?.updateValue(item.parentCategory.id, forKey: "parentCategoryId")
newItems["\(itemRef.key)"]?.updateValue(item.item.id, forKey: "itemId")
newItems["\(itemRef.key)"]?.updateValue(item.item.name, forKey: "name")
newItems["\(itemRef.key)"]?.updateValue(item.qty, forKey: "qty")
newItems["\(itemRef.key)"]?.updateValue(item.discountAmount, forKey: "discountAmount")
newItems["\(itemRef.key)"]?.updateValue(item.notes, forKey: "notes")
newItems["\(itemRef.key)"]?.updateValue(item.price, forKey: "price")

//set options
newItems["\(itemRef.key)"]?.updateValue([String:Dictionary<String,AnyObject>](), forKey: "options")
for option in item.options{
if var optionsRef = newItems["\(itemRef.key)"]?["options"] as? [String:Dictionary<String,AnyObject>]{
optionsRef.updateValue([String:AnyObject](), forKey: "\(option.item.id)")
optionsRef["\(option.item.id)"]?.updateValue(option.item.name, forKey: "name")
optionsRef["\(option.item.id)"]?.updateValue(option.group.id, forKey: "group")
optionsRef["\(option.item.id)"]?.updateValue(option.qty, forKey: "qty")
optionsRef["\(option.item.id)"]?.updateValue(option.price, forKey: "price")
}
}
}

Answer

The primary issue here is that when you say if var optionsRef = newItems["\(itemRef.key)"]?["options"] as? [String:Dictionary<String,AnyObject>], you're making a copy of the Dictionary, since Dictionaries are value types in Swift. As a result, all subsequent modifications of the dictionary are applied only to your local copy, and not the one you intended. To alleviate this, you must assign the modified version of the dictionary back into the data structure where you wanted it changed.

But in addition to that primary problem, there are many other, softer, issues with this code. There's a reason this question has 22 views and no answers. People see this, say "Nope!" and run away screaming. Here are some key points:

  1. Be consistent with your use of shorthand notation. [String : [String : AnyObject], not [String : Dictionary<String, AnyObject>].
  2. Don't use string interpolation ("\(foo)") solely to convert types to strings. If the values are already strings, use them directly. If they're not, convert them with String(foo).
  3. Don't use updateValue(_:forKey:). There's pretty much never a reason. Prefer subscript syntax.
  4. When initializing a complex structure (such as your dictionary), initialize it first, then add it. Don't add it, then repeatedly access it through cumbersome copy/pasting.
    • In your case, consider newItems["\(itemRef.key)"]?.updateValue(.... With every line, the key must be hashed, and used to retrieve the value from the newItems dictionary. Then, the returned value must be checked for nil (by the ?. operator), doing nothing if nil, otherwise executing a method. You know the value won't be nil because you set it, but it has to do all this checking anyway.
  5. Prefer Dictionary literals over mutation wherever possible.
  6. Don't use Foundation data structures (NSArray, NSDictionary, etc.) it's absolutely necessary.

Here's my take on this code. It might need some touch ups, but the idea is there:

var newItems = [String :  [String : AnyObject]() //fixed inconsistent application of shorthand syntax

let itemRef = ref.child("items").childByAutoId() //this shouldn't be in the loop if it doesn't change between iterations

for item in self.items {

    // There's pretty much no reason to ever use updateValue(_:forKey:), when you can just use
    // subscript syntax. In this case, a Dictionary literal is more appropriate.

    var optionsDict = [String : [String : AnyObject]();

    for option in item.options {
        optionsDict[option.item.id] = [
            "name" : option.item.name,
            "group" : option.group.id,
            "qty" : option.qty,
            "price" : option.price,
        ]
    }

    newItems[itemRef.key] = [
        "parentCategoryId" : item.parentCategory.id,
        "itemId" : item.item.id,
        "name" : item.item.name,
        "qty" : item.qty,
        "discountAmount" : item.discountAmount,
        "notes" : item.notes,
        "price" : item.price,
        "options" : optionsDict
    ]
}

Here's an alternative that avoids having to define optionsDict separately:

let itemRef = ref.child("items").childByAutoId()

for item in self.items {

    newItems[itemRef.key] = [
        "parentCategoryId" : item.parentCategory.id,
        "itemId" : item.item.id,
        "name" : item.item.name,
        "qty" : item.qty,
        "discountAmount" : item.discountAmount,
        "notes" : item.notes,
        "price" : item.price,
        "options" : item.options.reduce([String : AnyObject]()) { (var dict, option) in
            optionsDict[option.item.id] = [
                "name" : option.item.name,
                "group" : option.group.id,
                "qty" : option.qty,
                "price" : option.price,
            ]
        }
    ]
}