views:

47

answers:

2

How to succinctly handle this situation. I'm not properly releasing contactDictionary in the if statement...

    NSNumber *pIDAsNumber;
     ...        
    NSMutableDictionary *contactDictionary = [NSMutableDictionary dictionaryWithDictionary:[defaults dictionaryForKey:kContactDictionary]];
    if (!contactDictionary) {
            contactDictionary = [[NSMutableDictionary alloc] initWithCapacity:1];
    }
    [contactDictionary setObject:pIDAsNumber forKey:[myClass.personIDAsNumber stringValue]];
    [defaults setObject:contactDictionary forKey:kContactDictionary];
+2  A: 

Normally, use [NSMutableDictionary dictionaryWithCapacity:1] instead of alloc/init. That will give you an autoreleased dictionary that will, from a memory management perspective, behave identically to the one above. However...

In this specific case, your if clause will never be true (unless you run out of memory, in which case you have bigger problems). -dictionaryWithDictionary: returns an empty dictionary rather than nil if it is passed nil. So, even if -dictionaryForKey: returns nil, -dictionaryWithDictionary: will still create an empty mutable dictionary for you to add to.

Boaz Stuller
Yes. Instead, you should use `[[defaults dictionaryForKey:kContactDictionary] mutableCopy]`.
Rob Keniger
A: 

You can drop the if statement entirely, because -[NSMutableDictionary dictionaryWithDictionary:] always returns a dictionary.

Also, don't use [NSMutableDictionary dictionaryWithCapacity:1] to get an empty, autoreleased mutable dictionary. Just use [NSMutableDictionary dictionary].

Jeremy W. Sherman