6

When coding a method or function, is a good practice to check the input arguments to respond to any possible fail scenario.

For example:

-(void)insertNameInDictionary:(NSString*)nameString
{
    [myDictionary setObject:nameString forKey:@"Name"];
}

This looks ok, but if nameString is nil, the app will crash. So, we can check if it's nil, right?. We can also check if it is a NSString and not a NSNumber or if it is responds to the methods that our method needs to call.

So my question is: Which is the most complete and elegant way to check these arguments?

LuisEspinoza
  • 8,508
  • 6
  • 35
  • 57
  • 1
    What do you want to accomplish? If the name is a nil, what would be the proper response? – Hot Licks Jan 10 '14 at 20:56
  • What's wrong with putting a nil check on `nameString` or doing a `@try...@catch()` block and handling the exception there? You can make the argument that all of your parameters everywhere need to have a "complete and elegant" validation check. That seems like you'll never trust your inputs. I would argue that you need to guard against your inputs in a "complete and elegant" way when they're external to the program and just focus on writing "complete and elegant" business logic to ensure that you're not calling these methods with bad data to begin with. – Tim Reddy Jan 10 '14 at 21:04
  • @HotLicks I want to accomplish a proper input verification, to prevent fails. Checking if it is nil and checking for the class, would prevent my code to fail and (@T Reddy) prevents that my code cause failures in external code. So my questions is, can be more complete? (am I skipping other important checks for NSString or other clasess?) and can be more elegant? (do the system provides tools for this?, or, there is some kind of guidelines?) – LuisEspinoza Jan 10 '14 at 21:15
  • Again, what do you propose to do if the value is wrong? Why is it so important to blame the caller? (This is a reasonable thing to be concerned with if you're writing an API you're going to sell, but not so big a deal if you're your own client, or working in a small group.) – Hot Licks Jan 10 '14 at 21:20
  • @HotLicks is not my purpose to blame the caller. I usually try to write my code thinking like it is an API, thinking in possible client code that can accidentally pass incorrect arguments. In that case, if the value is wrong, I would at least expect that the app not crash and not to cause problems to other code. For example, if we are expecting to have a NSDictionary full with NSStrings, my example method should not insert a NSNumber, even if it is not nil. Inserting a NSNumber could produce a crash in the code of other developer (or even mine). – LuisEspinoza Jan 10 '14 at 21:31
  • 1
    But if you ignore the bad value then the bug goes undetected. Better to crash. – Hot Licks Jan 10 '14 at 21:33
  • Well you are right, would be better to "do something", in that case, I would NSlog an error, a crash seems to destructive, since the application could be doing important things in other threads, and maybe this failure could be tolerated. – LuisEspinoza Jan 10 '14 at 21:39
  • And who will look at the NSLog? – Hot Licks Jan 11 '14 at 03:35

5 Answers5

25

There are multiple ways to implement such protection:

  • NSParameterAssert
  • __attribute__((nonnull))
  • and a simple if test.

[EDIT] Since the latest Xcode versions (Xcode 6), Apple added nullability annotations which is another — and better — way to express if a parameter can be nil / null or not. You should migrate to that notation instead of using __attribute__((nonnull)) to make your APIs even more readable.


In details:

  1. NSParameterAssert is Apple's dedicated macro that checks a condition on a method parameter and throws a dedicated exception if it fails.

    • This is a protection only at Runtime
    • This macro still considers that passing nil as a parameter is a programmation/conception error, considering that due to your application workflow it should normally never happen (due to other conditions ensuring the param never being nil for example), and if it's not something really went wrong.
    • It still thows an exception (that your calling code may @try/@catch if necessary), but it has the advantage that the exception thrown is more explicit (telling that the parameter was expected not to be nil instead of crashing with an ugly and hard-to-understand callstack/message).
  2. If you want to allow your code to call your method with nil but just do nothing in that case, you may simply if (!param) return at the beginning of the function/method.

    • This considers that passing nil is NOT a programmation/conception error and thus can sometimes happen due to the workflow of your application, so that's an acceptable case that can happen and just shouldn't crash.
  3. Less commonly known, there is the __attribute__((nonnull)) GCC/LLVM attribute that is dedicated to tell the compiler that some parameters of a function/method are expected to be not-null. This way, if the compiler can detect at compile time that you try to call your method/function with a nil/NULL argument (like directly calling insertNameInDitionary:nil instead of using a variable which value can't be determined at compile time yet), it will emit a compilation error right away to let you fix it ASAP.

  4. [EDIT] Since latest Xcode 6, you can (and should) use nullability annotations instead of __attribute__((nonnull)). See examples in Apple's Blog post.


So in summary:

If you want to mark that your method EXPECT your parameter to be non-nil, thus indicating that calling it with nil is a logic error, you should do the following:

- (void)insertNameInDictionary:(NSString*)nameString __attribute__((nonnull))
{
    // __attribute__((nonnull)) allows to check obvious cases (directly passing nil) at compile time
    NSParameterAssert(nameString); // NSParameterAssert allows to check other cases (passing a variable that may or may not be nil) at runtime
    [myDictionary setObject:nameString forKey:@"Name"];
}

If you think that calling your method with nil can happen and is acceptable, and you just want to avoid a crash in those cases, simply do stuff like this:

-(BOOL)insertNameInDictionary:(NSString*)nameString
{
    if (nameString == nil) return NO;
    [myDictionary setObject:nameString forKey:@"Name"];
    return YES;
}

And if you think that you should be able to insert a nil object in your dictionary, you can convert the nil value to NSNull in that specific case so that it insert the NSNull singleton that is dedicated to such usage (I used the short form of the ?: ternary operator to make the code more compact):

-(void)insertNameInDictionary:(NSString*)nameString
{
    [myDictionary setObject:nameString?:[NSNull null] forKey:@"Name"];
}

And last case, if you want that passing nil in that particular example simply removes the Name from myDictionary, you can simply either do a simple if test and call removeObjectForKey:@"Name" if nameString is nil and call setObject:forKey: if it's not… or you could use KVC and the generic setValue:forKey: (KVC method, not specific to NSDictionary at all, so not to be confused with setObject:forKey:) that in the case of NSDictionary exactly has the same behavior (removing the key from the dictionary if we pass nil for the value parameter).

AliSoftware
  • 32,623
  • 6
  • 82
  • 77
  • 1
    Note that since this answer, Apple has added "nullability annotations" to the ObjC language, so instead of using `__attribute__((nonnull))` one should instead annotate their parameters using `nullable` and `nonnull` and use `NS_ASSUME_NONNULL_BEGIN` + `NS_ASSUME_NONNULL_END` region auditing macros. See https://developer.apple.com/swift/blog/?id=25 for more info. – AliSoftware Jun 17 '15 at 13:43
2
- (void)insertNameInDictionary:(NSString *)nameString
{
    if ([nameString isKindOfClass:[NSString class]]) {
        [myDictionary setObject:nameString forKey:@"Name"];
    }
}
Duncan C
  • 128,072
  • 22
  • 173
  • 272
Gavin
  • 8,204
  • 3
  • 32
  • 42
2

Apple suggests using NSAssert for that:

NSAssert(nameString, @"nil nameString is not allowed");

This assertion would terminate your program, and produce an error message explaining what has happened.

Above, the != nil part is implicit, because Objective-C allows it. You can state it explicitly for somewhat better readability:

NSAssert(nameString != nil, @"nil nameString is not allowed");

Assertions are not limited to nil checking, because they can take arbitrarily complex conditions. You can check that an argument is of the expected type, that it responds to a particular selector, and so on. Assertions can be disabled in release code to save CPU cycles and the battery.

Community
  • 1
  • 1
Sergey Kalinichenko
  • 714,442
  • 84
  • 1,110
  • 1,523
1

I see a couple problems here. NSMutableDictionary doesn't have a "setObject" method. You need to specify a key.

-(void)insertNameInDictionary:(NSString*)nameString
{
    if(nameString)
        [myDictionary setObject:nameString forKey: @"someKey"];
}

And you can also do a nil check via the "if(nameString)" bit there in my example. If you want to have an empty string, use @"" and not nil.

Michael Dautermann
  • 88,797
  • 17
  • 166
  • 215
  • Yeah, I see. You can't put a nil object in for a key. You need to do an empty string (`@""`) or NSNull or some Objective-C object that is ***not*** nil. Also, the NSDictionary needs to be **NSMutableDictionary**. – Michael Dautermann Jan 10 '14 at 20:45
  • This doesn't confirm if it's the right type or not. Because somebody could always pass in something that's not a string, and the poster asked about that. My answer below verifies it's a string first, in addition to making sure it's not nil. – Gavin Jan 10 '14 at 20:50
  • That's nice @Gavin. +1 to your answer. My answer just checks to see if the input is null. I could edit my answer to do both type checking and nil checking, but I think the Luis knows what to do now. – Michael Dautermann Jan 10 '14 at 20:51
  • Yes, maybe I did not communicate my question correctly. I'm asking for a way to check the input. The input could be a NSString, a NSArray or any other object. – LuisEspinoza Jan 10 '14 at 20:53
  • then Gavin's answer is the best answer for your question, Luis. – Michael Dautermann Jan 10 '14 at 20:53
1

it all depends on the semantics of what you want to happen, if you want the KV pair removed if the value is nil, then you can use setValue:forKey:

like:

-(void)insertNameInDictionary:(NSString*)nameString
{
    [myDictionary setValue:nameString forKey:@"Name"];
}

or if you want to store a value that represents a nil value you can use NSNull

-(void)insertNameInDictionary:(NSString*)nameString
{
    [myDictionary setObject:nameString? :[NSNull null] forKey:@"Name"];
}

-(NSString *)nameInDictionary
{
    NSString * retVal = [myDictionary objectForKey:@"Name"];
    return (retVal==[NSNull null]) ? nil : retVal;
}

or you may very well just want to log it.

-(void)insertNameInDictionary:(NSString*)nameString
{
    if(!nameString)
    {
        NSLog(@"expected nameString to be not null... silly me %s",__PRETTY_FUNCTION__);
        return;
    }
    [myDictionary setObject:nameString forKey:@"Name"];
}
Grady Player
  • 14,399
  • 2
  • 48
  • 76