1

I have the following bit of code where I'm getting a response from the server and trying to parse out error messages.

I notice that on occasion the message object isn't returning as type NSDictionary and will crash the application. I'm wondering what the best practice is to protect against that? In general I try to avoid doing instanceof checks. Likewise has selector checks. It feels like there should be a better way to do this than explicitly check I'm allowed to be using those methods / getting back type that I expect.

NSDictionary *message = [serverErrorJSON objectForKey:@"message"];
if (message != nil) {
return [message objectForKey:@"form"];
}
nazbot
  • 1,527
  • 2
  • 15
  • 23
  • 2
    Nope, checking to see if an object `respondsToSelector:` is the best way to see if it responds to a selector. However, it would be better practice to make sure your API returns consistent responses. – Alex Zielenski Aug 27 '14 at 22:53
  • Worth noting that the template system in Swift will solve this problem. Dictionary and Array elements will be strongly typed. – Jeff Holliday Aug 27 '14 at 23:27
  • Alex, that's definitely the first thought I had re: it's more an API problem at the core. Just figure doesn't hurt to also have some safety here (with assert or logging to notify when something slips through). Alex, that's good to know! Maybe should start transitioning over. :) – nazbot Aug 27 '14 at 23:41

3 Answers3

4
if ([serverErrorJSON isKindOfClass:NSDictionary.class]) {
  return serverErrorJSON[@"message"][@"form"];
}

return nil;

Its good to use the literal syntax both syntactically and programatically.

You don't have to chain them together either, you might want to use another property of the JSON,

if ([serverErrorJSON isKindOfClass:NSDictionary.class]) {
  NSDictionary *message = serverErrorJSON[@"message"];
  //...
  return message[@"form"];
}

return nil;
Oliver Atkinson
  • 7,970
  • 32
  • 43
  • Out of curiosity, what's the advantage of using the literal syntax? – nazbot Aug 27 '14 at 23:43
  • Of course, for certain cases you might want to check `conformsToProtocol` or `respondsToSelector` instead of using `isKindOfClass`. – Hot Licks Aug 27 '14 at 23:53
  • 1
    @nazbot see: http://stackoverflow.com/questions/12535855/should-i-prefer-to-use-literal-syntax-or-constructors-for-creating-dictionaries – Oliver Atkinson Aug 28 '14 at 11:11
0

Your code:

NSDictionary *message = [serverErrorJSON objectForKey:@"message"];
if (message != nil) {
    return [message objectForKey:@"form"];
}

The reason you are getting "unrecognized selector" here is that [serverErrorJSON objectForKey:@"message"] is not returning an NSDictionary as you expect. Since you are calling objectForKey: on that object, the recommended way to handle this is to wrap that call with respondsToSelector: :

id message = nil;
if ([serverErrorJSON respondsToSelector:@selector(objectForKey:)]){
    message = [serverErrorJSON objectForKey:@"message"];
    if ([message respondsToSelector:@selector(objectForKey:)]){
        return [message objectForKey:@"form"];
    }
}

This tests for the presence of the selector (method) you are calling, and is safer and more compatible than using isKindOfClass: and the like. It's always better to test for capabilities rather than class names, etc. You don't care if the object is an NSDictionary, you care if it can provide an object for a key using the method signature you are invoking.

quellish
  • 21,123
  • 4
  • 76
  • 83
-1

The best way to protect against unrecognized selector errors is to check respondsToSelector or isKindOfClass depending on your use case.

Unfortunately your code can get pretty cluttered if you find yourself needing to verify that an object is a dictionary frequently, which can occur in your situation like yours with nested dictionaries in the data structure.

You can clean things up by adding a category:

@interface NSDictionary (Safe)

//Returns objectForKey if dictionary param is valid, else returns nil
+ (id) _safeObjectForKey: (NSString*) key
                    dict: (NSDictionary*) dictionary;

@end

@implementation NSDictionary (Safe)

+ (id) _safeObjectForKey:(NSString *)key
                    dict:(NSDictionary *)dictionary {

    if ([dictionary isKindOfClass:[NSDictionary class]]) {
        return [dictionary objectForKey:key];
    }
    return nil;

}

@end

and then to use it, given your example:

NSDictionary *message = [NSDictionary _safeObjectForKey: @"message"
                                                   dict: serverErrorJSON];
return [NSDictionary _safeObjectForKey:@"form"
                                  dict:message];

You can do something similar for other common unrecognized selector error generators as well, like NSArray's objectAtIndex etc.

BFar
  • 2,447
  • 22
  • 23
  • Can someone explain the -1 for this? I completely understand that the approach I've taken is unconventional, but it has worked for me and creating convenience category methods like these help me write code faster & reduce the # of lines needed. No worries if I deserve a -1, I'd just like to understand why so I can improve my own coding habits. If you dislike this approach, please give me feedback! – BFar Sep 06 '14 at 03:48