Correct Structure to check for Errors using NSError

Question

I'm coding up various routines and I'm trying my best to keep it neat and refactored.

Methods I'm creating are starting to look similar to this code:

-(IBAction)buttonPress:(id)sender {
    // Create Document Shopping List with this document
    [self doSomething:&error];

    if(error) {
        [NSApp presentError:&error];
        return nil;
    }

    [self doSomethingElse:&error];

    if(error) {
        [NSApp presentError:&error];
        return nil;
    }

    [self doYetSomethingElse:&error];

    if(error) {
        [NSApp presentError:&error];
        return nil;
    }
}

I love NSError, but this seems like an awfully clumsy way of handling all my errors.

A few thoughts I've had about alternative ways:

a) the error checking could be built into the methods doSomething, doSomethingElse etc, but then I wouldn't be able to exit the button press method without doing some kind of checking on the return value, which would lead me back to a similar structure.

b) I could set up the NSError as being Key Value Observed, but something about this feels deeply wrong. I'm very aware of the possibilities of abuse with KVO, so I'm trying to do everything without it whereever possible.

Surely I'm missing something really basic here? Is there a pattern that can help me? Or is this structure OK?

Answer 1

Note: When I posted this I was unaware of any MacOS/Cocoa exceptions debate. Please bear this in mind when you consider this answer.

Why not use exceptions instead? This will allow you to dispense with the error parameter on your methods and handle your errors in one place.

-(IBAction)buttonPress:(id)sender {
    // Create Document Shopping List with this document
    @try {
        [self doSomething];
        [self doSomethingElse];
        [self doYetSomethingElse];
    } @catch (NSException* e) {
        [NSApp presentException:e];
    }
    return nil;
}

You will of course need to throw exceptions as required from within your doXXXX methods:

NSException* e = [NSException exceptionWithName:@"MyException"
                                         reason:@"Something bad happened"
                                       userInfo:nil];
@throw e;

Answer 2

Hey John,

I think the real question is how much granularity you need in your error handling. Your code is fundamentally correct - but you can clean things up by checking for errors less frequently, or using a catch-all approach like teabot mentioned in his answer with NSException. A lot of the built-in functions that return errors (like NSFileManager's moveItemAtPath:) return a BOOL in addition to a providing an NSError object - so you could also use nested if statements if you don't need the NSError information.

All and all though, there's really not a great way to do this. If your code is long, KVO might be a cool solution. I've never tried it for a situation like this, though.

Answer 3

The gist of your question is whether there are structural improvements you can make to your error handling. I think so, by essentially introducing more layers of nesting, either by extracting more code into separate methods/functions, or by introducing nesting in your high level sample method.

The idea is, when it comes to handling most errors, you probably are either interested in performing an alternate task, or in failing and propagating the error up the chain so that some responsible controller can convey the error to the user through UI.

Using this idea of "propagate or handle", I would rewrite your sample method like this:

-(IBAction)buttonPress:(id)sender {

    // Create Document Shopping List with this document
    [self doSomething:&error];    
    if(error == nil) {
        [self doSomethingElse:&error];
        if (error == nil) {
            [self doYetSomethingElse:&error];
        }
    }

    if(error) {
        [NSApp presentError:&error];
    }    
}

Note that there are good arguments against introducing too much nesting in a particular method. Nesting such as this is essentially a short alternative to extracting methods. It might make more sense, for instance, that "doSomething:" itself calls doSomethingElse:, which calls doYetSomethingElse: for instance. This would impose the same structure on the code as the if-nest, but would be arguably more maintainable.

As an aside, I am not a fan of inline return statements. In this particular instance, the sample method doesn't actually call for a return value, but if it did, I prefer to set a local variable to the returned value and only return at the end of the flow control. Jumping out of a function or method prematurely is a sure way to encounter weird bugs, IMHO.