As You Were

Devin Coughlin's blog.
Styles: Serious Spare

April 14, 2007

-[NSDictionary dictionaryWithObjectsAndKeys:] considered harmful

Yeah, the title is over-used, but -[NSDictionary dictionaryWithObjectsAndKeys:] will seriously bite you in the ass if you aren't careful.

NSDictionary is Apple's implementation of a hashtable (or associative array) in Objective-C. It's pretty standard stuff; you can store and retrieve objects keyed on arbitrary (often string) values, lookup is of constantish time complexity, etc. The only real gotcha is that the key value has to support immutable copying (i.e. -copyWithZone:) because NSDictionary doesn't want the key changing underneath it.

One particularly common use for NSDictionary is as a container tying together a bunch of disparate objects. This way you only have to cart around a single object reference.

A common way to create a dictionary in Cocoa is this:

NSURL *url = @"http://www.google.com";
NSImage *favicon = Some Image
NSString *title = @"Google";

NSDictionary *bookmark = [NSDictionary dictionaryWithObjectsAndKeys:url, @"URL", favicon, @"FAVICON", title, @"TITLE", nil];

Here the method -[NSDictionary dictionaryWithObjectsAndKeys:] takes your standard C-style variable argument list. Since varargs in C are brain dead and don't know how may arguments they have (*) you pass nil as the last argument to delimit the end of the list. This isn't ambiguous because you can't put nil in an NSDictionary. You end up with a dictionary that looks like this:

bookmark = { URL => http://www.google.com/,
  FAVICON => Some Image,
  TITLE => "Google" 
}

While this incantation is concise and very convenient, it is going to kick your ass someday — and you might not even know it.

What happens if favicon is nil?

If favicon is nil, then you're essentially creating the bookmark dictionary as follows:

NSDictionary *bookmark = [NSDictionary dictionaryWithObjectsAndKeys:url, @"URL", nil, @"FAVICON", title, @"TITLE", nil];
Since that first nil is interpreted as the end of arguments, the dictionary looks like:
bookmark = { URL => http://www.google.com/
}
The "title" key-value pair is missing. That's right — silent data loss. Oops.

A safer, but much more clunky way to construct a dictionary is:

NSURL *url = ...
NSImage *favicon = ...
NSString *title = ...

NSMutableDictionary *bookmark = [NSMutableDictionary dictionary];

if (url != nil)
    [bookmark setObject:url forKey:@"URL"];
    
if (favicon != nil)
    [bookmark setObject:favicon forKey:@"FAVICON"];
    
if (title != nil)
    [bookmark setObject:title forKey:@"TITLE"];
Ick, right? Not just more lines of code, but more codepaths. But no silent data loss. And if you expect your objects to be non-nil, you can leave out the conditionals because -[NSDictionary setObject:forKey:] will throw an exception if the object is nil.(**)

After recently tracking down several bugs to just this kind of problem, I've promised myself I'm never going to use -[NSDictionary dictionaryWithObjectsAndKeys:] again.

There is a similarly pernicious problem with -[NSArray arrayWithObjects:] and all other containers that use nil-terminated varargs, but, for whatever reason, I've found that the dictionary case is the one that always gets me.

(*) The stupidity with which C handles varargs leads to all sorts of nasty bugs, including format string attacks.

(**) The fact that you can't set an object to be nil in an NSMutableDictionary has the odd consequence that

[dictionary setObject:[dictionary objectForKey:@"FOO"] forKey:@"FOO"]
doesn't always work.

Posted by coughlin at 11:49 PM | Comments (1)
Comments

I've been bitten by this as well, but still use -dictionaryWithObjectsAndKeys: -- I find it too convenient. I work-around the nil-value problem with a little nilIsNull() function to wrap values that I pass in. Something like this:

static id nilIsNull(id value) { return value ? value : [NSNull null]; }

[NSDictionary dictionaryWithObjectsAndKeys: nilIsNull(url), @"URL", nilIsNull(favicon), @"FAVICON", nilIsNull(title), @"TITLE", nil];

It still sucks, but is about as good as you can get with ObjC.

Posted by: rentzsch at April 15, 2007 11:44 AM