Using NSMutableDictionary instead of NSDictionary about io2015-codelabs HOT 5 CLOSED

Hmmm... not sure about this one. For learning purposes, it's probably good for folks to understand that GAIDictionaryBuilder returns an NSMutableDictionary.

Also, I'm not sure casting it as a NSDictionary will change the fact that underneath it's still an NSMutableDict. If you add this line under your modified code...

NSLog(@"Event is mutable dictionary? %d", [event isKindOfClass:[NSMutableDictionary class]]);

you'll see that it's still considered an NSMutableDictionary by iOS.

On the other hand, if your bug is really suggesting that the API is wrong and it should be returning a dictionary instead of a mutable dict, that's another matter. :) I don't know how often we expect developers to make calls that would modify this thing after the fact...

from io2015-codelabs.

I'm going to close this one as fine. The Analytics docs (not just the codelab) suggest using NSMutableDictionary since that's the real type.

from io2015-codelabs.

@ToddKerpelman I don't see reasons why the code can not look like this:

NSDictionary *event =
[[[GAIDictionaryBuilder createEventWithCategory:@"Action"
action:@"Share"
label:nil
value:nil] build] copy];
[tracker send:event];

This is more understandable and safe.

from io2015-codelabs.

That's a fair point.

I actually think this probably points to a larger issue, which is: should this be a mutable dictionary in the first place? It all depends on whether or not the developer is ever meant to change things with it afterwards. (Or if it's a requirement from the library because it needs to somehow mutate it afterwards.) If not, maybe the right solution would be to file a bug and just have this method return an NSDictionary.

from io2015-codelabs.

FWIW, I'm bringing this up with the Analytics SDK team to see what they have to say.

from io2015-codelabs.