elegant chaos

Simon Wolf wrote a blog post recently talking about passing NSError pointers into methods like this:

NSError* error = nil;
[something someMethod:@"blah" error:&error];

He talked about the fact that when you do this, you should always check the result of the method before using the error. In other words, you do this:

NSError* error = nil;
if (![something someMethod:@"blah" error:&error])
{
	NSLog(@"got error %@", error);
}

and not this:

NSError* error = nil;
[something someMethod:@"blah" error:&error];
if (error)
{
	NSLog(@"got error %@", error);
}

This is good advice. The method you’re calling isn’t guaranteed to set error to a non-nil value if nothing goes wrong. It might leave it alone, but it might set it to a temporary value, even though it eventually returns an “ok” result.

However, Simon then went on to talk about this line at the top:

NSError* error = nil;

His assertion was that by setting error to nil, you’re indicating that you intend to test it after the call - in other words that it’s an indicator that you don’t know what you’re doing.

He suggested that you should just do this:

NSError* error;

Strictly speaking, this is correct. If the method you’re calling follows the implicit contract for such functions, this should be fine.

However, I always set the error to nil before I pass it in, even though I never test it after the call. Why do I do this?

Consider the following method:

- (BOOL)someMethod:(NSString*)string error:(NSError**)error
{
	if (error)
	{
		if (something)
			*error = [self doSomeStuff];
		else (somethingelse)
			*error = [self doSomethingElse];
		//... lots of other cases here...
		
		//... later
		
		if ([(*error) code] == someCode)
		{
			[self doSomeExtraStuff];
		}
	}
	
	return YES;
}

This is a bit of a contrived example, but the point is that this method might be the one you’re calling, and it might be in a third party library that you have no control over.

Now lets imagine that some refactoring or other code change introduces a bug where *error doesn’t get set in the first block of if/else statements. Let’s also imagine that the developer has the relevant warnings turned off, and doesn’t noticed (shock horror - some people actually do this!).

What happens when the code starts accesses *error? Well, it depends what’s in it. If you did this:

NSError* error = nil;

you’ll be fine.

If you did this:

NSError* error;

you get undefined behaviour, based on what happened to be on the stack in the position that the error variable occupies. It’s a classic uninitialised variable bug, and potentially a bastard to track down.

At this point you may well be saying “if the quality of the library you’re using is that bad, you’ve only yourself to blame”. You might have a point, but bugs do creep in, even to good libraries.

Admittedly too, when you’re accessing an uninitialised Objective-C pointer, you’re way more likely to crash straight away than you would have been if you were just dereferencing a pointer to read a member in C/C++.

However, all of this is the reason why I still do:

NSError* error = nil;

even though I’m not going to test error afterwards.

You could call it paranoia, but don’t mistake it for misunderstanding the NSError** contract!