Ever seen this hard to notice bug in an iPhone / Mac project? Caused by a flaw (bug?) in Apple’s own API:
NSDictionary * map;
…
NSObject * key;
NSObject * value;
…
// NB: next line will frequently crash at runtime
// …but even if doesn’t crash, it probably won’t do
// what you’d expect it to:
[map setObject:value forKey:key];if( value != [map objectForKey:key] ) // HA!
{
NSAssert( 1 == 0, @”What the **** just happened??” );
}
NSDictionary is one of the nastier things I’ve seen in a programming language: in an Object-Oriented Language, it’s a class that refuses to take Objects as arguments, *but pretends to*. If you attempt it, it either crashes, or it invalidates your objects, breaking contracts all over the place.
ObjC’s bizarre design
In the days pre-OOP, a “dictionary” was something that mapped:
“a string”
to
“anything” (usually: basic datatypes – e.g. strings, integers, floats, etc).
In the days of OOP, the same thing is usually called a “map” (which is a better term) – although the terms are synonymous – and maps:
“an object”
to
“another object”
What did Apple/NextStep/Crazy-Guys-Behind ObjC do?
NSDictionary: maps:
“STRINGS ONLY” (no objects allowed!)
to
“OBJECTS ONLY” (no core datatypes allowed!)
But … I can use an NSObject as key?
Yep – but Apple’s internal implementation takes a *copy* of the object, and uses that as a key – rather than using the object that you gave it. This is a common problem in OOP languages and implementations of Map – e.g. Java does the same thing.
Unfortunately, this means that you can call “setObject:forKey:”, and then “objectForKey:” will return nil *for the same key*.
In Java and other OOP languages, you are required to implement a custom “isObjectEqualToObject” method. In ObjC too – except that that method is ILLEGAL if you’re using CoreData.
And ObjC will crash too, as a bonus
I’ve never seen this in other OOP languages, but in ObjC *additionally*: if you don’t manually add
How come? AFAICT, Apple’s header file is wrong:
– (void)setObject:(id)anObject forKey:(id)aKey // You lie!
it seems the implementation of that method is:
– (void)setObject:(id)anObject forKey:(id<NSCopying>)aKey; // the correct signature?
Net result? Code that happily compiles … will crash. ARGH!
Two reasons to beware…
…so what’s the other one?
Ah, just the one we see again and again on live projects, wasting hours and hours of time:
NSDictionary* dictionary = [NSDictionary dictionaryWithObjectsAndKeys:
object1, key1,
object2, key2,
nil];NSLog( @”dictionary = %@”, dictionary ); // but it only has one key/value pair. ?!?!?
Ah, well … that nil … what happens when object2 is nil? Oh, damn.
What about if key2 is nil? Now we’re really nasty … we’ve given it “half” of key/value pair. Nice!
5 replies on “2 reasons to beware Apple’s NSDictionary, and CoreData”
On the other hand, the fact that the key is copied is documented in the NSMutableDictionary class reference, and the Collections Programming Topics document. Have you filed a bug report against the header file?
I think CFDictionary can be configured with the no-copy semantics that you were expecting for the key values. And since those are zero-cost bridged to NSDictionary, the only hassle should more lines of code to initialize the dictionary.
And as far as +dictionarywithObjectsAndKeys: goes, silliness concerning extra nil values happens in all varargs methods of that style. :-/
@Brad – yep, agreed, hence title was “to beware” rather than “to avoid” ;).
Sadly, for every 1 programmer who is fully aware of those facts, I’m guessing there’s at least 1 more equally good programmer who is not. Primarily because this is such a bizarre facet of the language – anyone accustomed to normal languages is unlikely to notice it (until it bites them in the ass).
Re: CFDictionary – yep, learning to use that instead is IMHO one of the best (read: “fastest, easiest, least effort, and easiest for other coders to understand what you’ve done”) workarounds. But … is it worth converting other people’s codebases when you join the project, to remove the NSdicts for all except read access? Maybe … personally, I’ve never bothered, but I’m beginning to think it might be worth it. I keep seeing bugs like this on other people’s codebases, bugs they’ve been unable to fix.
The nil varargs thing – yep, but it tends to happen rarely in other places. Apple (and many library authors) force the use of NSDictionary for passing around parameters in lots of places – often there’s no alternative method, there’s only a version that takes an NSD as arg. And so you VERY often see the inline creation of dictionaries, just to make up for the API design (which is itself probably making up for the language design, where often the only easy way to handle multi-arg stuff is to use the NSDictionary. Varargs is only easy if you’ve been a C programmer for yeasr and have memorized the bizareness of it ;)).
So … IME it’s about 20x as common to use varags with a dictionary as “all other uses combined”. Shrug.
Well, the post just comes off a bit negative when all it contains is complaints about how NSMutableDictionary works, and doesn’t end with “And here’s an alternative that’s quick, easy, and has less surprises: CFDictionary. ” ;)
For my part, the fact that NSMutableDictionary copies its keys is defensible because it’s the end result of a conversation with “Because the idiots keep passing in mutable keys, and their hash codes keep changing, …” A quick, easy design decision to make a bunch of problems go away twenty years ago, and then it turns out that everyone else decides otherwise. :-/
And I may have masked a certain amount of rage using the word “silliness” when describing the other vararg style methods (like +arrayWithObjects:, etc.) that iterate until they find the first nil argument. >:-( If you want to complain about the API design decision most likely to cause bugs and crashes if anything at all goes wrong, combining “iterate until nil” with “if anything goes wrong, return nil” in various constructors. But it’s not NSDictionary’s fault. :-(
“Yep – but Apple’s internal implementation takes a *copy* of the object, and uses that as a key – rather than using the object that you gave it. This is a common problem in OOP languages and implementations of Map – e.g. Java does the same thing.
Unfortunately, this means that you can call “setObject:forKey:”, and then “objectForKey:” will return nil *for the same key*.
In Java and other OOP languages, you are required to implement a custom “isObjectEqualToObject” method.”
I think there are a few issues about this statement:
– Java does not make a copy of your object when using it as key. (Referring to most widely used Map implementation, util.HashMap)
– The default implementation of equals() method of Object checks for equality based on identity thus if you have not overriden this; it would work as you expect it to work. An object “o” will be able to put / retrieve values correctly unlike your Obj. C example.
– Most basic classes have sensibly written equals() methods. E.g. two separate String objects; which both have the same value “str”, will map/hash as equal.
I can’t really see the problem here about Java.
In saying “java does the same” I was being flippant. I really meant “Java has a similar gotcha in that if you start putting brand new class instances into a Map as keys, you get strange behaviour when trying to later retrieve them, if you didn’t ALSO manually implement appropriate hashCode() and equals() methods”.
The Java approach is a LOT safer / better than the ObjC approach. But I was trying to point out it’s commonplace for OOP languages to make dictionary/map less than 100% automatic.