You're Doing It Wrong #3: error:NULL

This week's post is going to be short and simple, and it relates to error handling.

In Objective-C, a common way to report error information to the caller of a method is to use an NSError output argument. For example, here is the declaration of the writeToFile:options:error method on NSData:

- (BOOL)writeToFile:(NSString *)path
            options:(NSDataWritingOptions)mask
              error:(__autoreleasing NSError **)errorPtr;

The method returns a boolean to indicate success or failure. On failure, a pointer to an NSError object will be written to the error output argument (note the double stars). All very standard.

What I see too often is code that simply passes NULL to the error argument and doesn't bother to check the return value. This ignores or hides any errors that may be generated by the method:

NSData *data = [self makeData];
[data writeToFile:outputFile
          options:NSDataWritingAtomic
            error:NULL];

In my opinion this is just as bad as gobbling up exceptions in Java:

try {
    doSomething();
} catch (Throwable e) {
}

Swallowing errors like this is very much frowned upon. It can lead to nightmares in terms of maintainability and debugging, especially in a language like Objective-C where nil return values can go unnoticed and delay the symptoms of an error.

Laziness

I think the reason so many developers fall into this habit is due to laziness. In order to do things properly, you have to declare a new variable, wrap the method you are calling in an if statement, and somehow deal with the error. You may decide you have to pass the error up a level, which would require you to add error feedback to your own interface.

These annoying details crop up when you are already half-way through writing the method call, so it's almost understandable that people get frustrated and stuff in a NULL to get it over and done with.

It's still wrong, however. And you know it's wrong. So please, for the sake of professionalism and your fellow developers, get into the habit of always providing a value to the error argument. At the very least you should be logging the error:

NSData *data = [self makeData];
NSError *error;
if (![data writeToFile:outputFile
               options:NSDataWritingAtomic
                 error:&error]) {
    MyLogError(@"%@", error);
}

I don't even care whether or not you describe what you were doing in the message. Just stuff a @"%@" in there and get it done as quickly as possible. At least the error will be seen. This is the absolute minimum, and you should do this for every method call that provides error information.

Even if you are writing quick prototype or test code, I would also suggest you follow this rule. Indeed, these are the situations where you are most likely to make mistakes, and breaking into the debugger to see which method failed is a huge waste of time. It is much faster to write the code properly and get quick feedback on any problems.

Rules are there to be broken

Yes yes, OK. Rules are there to be broken, so what follows is one example where it may (may) be acceptable to ignore an error:

- (void)copySourceFileToDestination {
    NSString *source = [self sourcePath];
    NSString *destination [self destPath];

    NSFileManager *manager = [NSFileManager defaultManager];

    // Delete/overwrite destination
    // ignore error because destination might not exist
    [manager removeItemAtPath:destination
                        error:NULL];

    NSError *error;
    if (![manager copyItemAtPath:source
                          toPath:destination
                           error:&error]) { 
        MyLogError(@"Error copying file: %@", error);
        // ...
    }

    // ...

}

In this example we want to copy path source to destination, overwriting destination if necessary.

The copyItemAtPath:toPath:error: method will fail if destination already exists. To get proper overwriting behaviour we either have to implement NSFileManagerDelegate, or instead (as in this example) delete destination first with a call to removeItemAtPath:error.

In all likelihood, destination will not exist, so the call to removeItemAtPath:error is going to fail. We don't care about this particular failure case, so we ignore it. If anything else does go wrong, we still check the result of copyItemAtPath:toPath:error:, so we seem to be in the clear.

Implementing methods that return errors

I'm going to close this off by giving a little tip for when you write your own methods that return error information.

If somebody calls your code with a NULL error value, you want to be as helpful as possible and log the error for them:

- (BOOL)myAPIMethod:(__autoreleasing NSError **)errorOut
{
    // do some work
    // ...

    if (weGotErrors) {
        if (errorOut) {
            *errorOut = theErrorWeGot;
        } else {
            MyLogError(@"Error doing something: %@",
                theErrorWeGot);
        }
    }

    // ...
}

If we are provided with a non-NULL output argument, we write out the error and it is up to the caller to log, handle or ignore the error as they wish.

If we are given NULL, we're going to assume the developer is lazy and log the error for them. If they really want to ignore the error, they can provide an output argument and do the ignoring in their own code. In our code we are going to be responsible developers and make sure errors are visible by default.

Be responsible

Things can get tedious in Objective-C. Error handing can be annoying and can make an already ugly block of code even uglier. But stay true to your core values, be responsible, and put in that extra bit of effort to write robust and programmer-friendly code.

That's the end. Thanks for reading.