Today I decided that I had enough of the weird looking if statements I found that didn't quite "feel right" in the MagicalRecord codebase. Here's an example of what I mean:

+ (void) handleErrors:(NSError *)error
    if (error)
       /// do some error handling here

Now, when this method is called, it is referenced via the MagicalRecord class as such:

[MagicalRecord handleErrors:error];

This seems clean enough, and it works, and it's a single line of code everywhere in the MagicalRecord codebase, so big deal, right? Well, that if (error) statement has mentally bothered me for quite some time, but as all good pragmatic engineers, I left it alone. After all, it was code that worked. But I decide that I would change things up to see how it would turn out.

First, I had to reconcile with myself why this if statement felt wrong. If we look back at where this method is housed, it's in the MagicalRecord class. And we're dealing with errors. The MagicalRecord class is intended to handle MagicalRecord "stuff", like building the Core Data Stack™, providing access to background save methods and other related things. That the MagicalRecord class (even via a category itself) was handling errors felt wrong. So, now that I know what felt wrong, how do I fix it? I need to find this code a new home. What class deals with errors? How about NSError? That seems like a better place.

The other problem I noticed is with the handleErrors: method itself. It is more or less a function attached to a class object. Why do I have one object doing the work of another? Again, it's a symptom of the error handling code not living in the correct place in code.

After deciding it's new home, I added a new MagicalRecordErrorHandling category on NSError, and added an instance method MR_log. What this method will do is log the error to the console, which is basically the exact same functionality as before. But now, we've tied the logging functionality to an instance of an NSError. This means that when we have code like this:

NSError *error = //capture error from somewhere
[error MR_log];

when error is nil, MR_log is a message sent to a nil variable, and thus is ignored. Rather than having an explicit if statement prevent the error logging code from running (and dumping out empty log messages), the fact that no error is present effectively achieves the same result.

Now, is this a big performance boost? Certainly not. Is this going to make my code shorter? Maybe. But I am more comfortable and happy in knowing that the error handling code is in a more appropriate home, and as a pleasant side effect, the nasty if statement is now gone.

You can have a look at the code change on the MagicalRecord repo on github

Blog Logo

Saul Mora



Saul Mora

iOS Developer, Engineer, Dad, Human

Back to Overview