Delicious Monster
Pimp My Code, Part 4: Returning late to return early
First off, I apologize for how long it's been since my last post. It's been a bit nutty around Delicious Monster these last couple weeks, and I had to get a 1.5.2 release out as well. Also, my main machine lost its hard drive, which took two weeks to restore and repair. Also, I got the flu really bad. Also, I started dating again, which is one of those things you'd think I'd learn to stop doing, since by definition every relationship, no matter how short or how long, ends in pain.Anyways, if you've read this series from the beginning, you'll know that, besides being perenially single, I also insist on returning early from functions/methods/what-have-you, instead of nesting ifs inside ifs inside ifs. To me, this makes the code much quicker to understand at a glance -- the normal path is the one down the left column, and any if statements I see are just dealing with error conditions. I demonstrate this in an earlier column, so I won't paste it again here.
Though I will say, again, that the goal of good coding is to make your code apprehensible at a glance. That's the goal, the as in only. I've spent a 25 years learning this rule, and I'll keep harping on it until St. Peter hands me a different harp to play. (Then, I plan to play "Stairway to Heaven".)
Now, several people have asked a thorny question, including one perceptive man named Tobias, who I will quote, in part, here:
I do have a problem with your solution though, and I can't figure out a way to solve it, so I thought I might ask you and see if you have an answer. What if you want some specific code that's executed when bailing? Or say, some code that should always be executed after some statements to clean up? I can't use return; then to bail, because I need that code to be executed before returning.
Ah, young Tobias. Condescending comment. Witticism!
Actually, to be honest, I don't have a really good solution to this one. I have solutions I use, depending on the situation, but none of them are particularly pleasing. I still like them better than nested-ifs, and I've usually found that when unrolling nested-ifs, there were lots of ways the code could have failed and not been handled correctly; that is, nesting ifs is actually not a particularly robust way to handle errors.
So, here are some possible situations, and possible solutions to them:
Situation 1 is if you've got a couple of lines of code you'de like to execute before you return from several different places, but you don't need to put the return statement itself in those lines; eg, the return is simple enough that it's not a hardship to copy and paste it. For this we use an inline function. It's a good fit because it's scoped right (won't be visible from anywhere but inside this method), and it inherits the scope above it, so you can use "self" and access your instance variables.
This particular snippet is from the code I use to recognize whether a keypress that's coming in to the program is part of a scanner's input or if it's being pressed by a human, since some scanners ("wedge-type") just send USB keypresses as if they were just another keyboard, and it's freaking hard to figure out where a keypress came from in AppKit. Oh, sure, they claim there's a way, but I will literally give $200 to the person who can show me how to take a keypress NSEvent and get the device that it came from out of it. No B.S., cash money. (Or an interview!)
| Situation 1: Couple lines of cleanup code before returning |
inline void _sendAllEventsIncludingThisOne() { [self _sendNormallyStoredNumericKeypressEvents:nil]; [super sendEvent:theEvent]; } unsigned int modifierFlags = [theEvent modifierFlags]; // scanners never press the shift or command keys... if ((modifierFlags & 0xffff0000) != 0) { _sendAllEventsIncludingThisOne(); return; } unsigned int length = [[theEvent characters] length]; if (length != 1) { // WJS: pretty much all keydown and keyup events have exactly one character these days _sendAllEventsIncludingThisOne(); return; } |
Well, it's not pretty, but it does the job. One nice thing to note about this kind of approach is you can have different little "deconstructor" inline functions for dissassembling different pieces of a whole that you are assembling, so you could stack them intelligently if you get further along and fail, and thus require more unrolling actions. If you need to return some complex value when you bail, have your inline method return it, and then just call something like "return _sendAllEventsIncludingThisOne();"
In Situation 2 we see a situation where we have one exit point, and we pretty much always want to do the same thing in front of it.
| Situation 2: One exit point at end with cleanup code |
{ autoreleasePool = [[NSAutoreleasePool alloc] init]; if (![self _drinkIsFrosty]) // this can allocate a lot of memory goto bail; if ([self _drinkType] != wantedDrinkType) goto bail; // ingest drink here bail: [autoreleasePool release]; } |
Remember that this technique isn't a panacea: if you set up some other state in this method that needs to be unrolled, you're going to have to deal with it.
Here are some other real-life macros I've written. These could have easily been inline function, but I wrote them as macros because when I wrote them I wasn't sure if I was going to insert return statements into them; if you return from an inline function you just pop back to the method that had the error, but if you return from a macro you are actually returning from the containing method. This is sometimes a good thing, and sometimes a bad thing, and you have to decide which you want.
Note that these are the macros I use when dealing with QuickTime, so I can be notified if an error occurs and then decide if it's critical enough that I want to throw an exception. The problem with QuickTime's error model is there are a lot of errors that aren't critical and won't affect whether the program will continue correctly, so you have to sort your way through the values by trial and error at run time to figure out what errors you'll see in practice and what they mean. Oh, man, I sure DO NOT MISS this kind of programming. Bring on the Cocoa QuickTime!
| Situation 3: How I deal with Carbon APIs |
#define NoteErr(x) { if ((x) != noErr) NSLog(@"QuickTime note error %d in file %s on line %d", (x), __FILE__, __LINE__-1);} #define PedanticErrorCheck() { OSErr _err = GetMoviesError(); if (_err != noErr) NSLog(@"QuickTime warning error %d in file %s on line %d", _err, __FILE__, __LINE__-1);} #define BailErr(x) { if (x != noErr) [NSException raise:[NSString stringWithFormat:@"%d", x] format:@"Raised Carbon error %d in file %s on line %d", x, __FILE__, __LINE__-1];} ... { ... sequenceGrabberComponent = OpenDefaultComponent( SeqGrabComponentType, 0); if (sequenceGrabberComponent == nil) BailErr(-1); err = SGInitialize(sequenceGrabberComponent); BailErr(err); // specify the destination data reference for a record operation -- tell it we're not making a movie // if the flag seqGrabDontMakeMovie is used, the sequence grabber still calls your data function, but does not write any data to the movie file // writeType will always be set to seqGrabWriteAppend err = SGSetDataRef(sequenceGrabberComponent, 0, 0, seqGrabDontMakeMovie); BailErr(err); // create a new sequence grabber video channel err = SGNewChannel(sequenceGrabberComponent, VideoMediaType, &sequenceGrabberChannel); BailErr(err); ... } |
As I said, this is work-in-progress. None of these techniques are super-beautiful, but when I compare my sequence-grabbing code to the samples on the websites, I certainly think this is a HUGE improvement. Especially since usually the examples just say, "/* if there's an error, you should handle it. */" Yah, thanks, guys. We CERTAINLY would NOT want an example of how to tear down these structures that are in a primordial state.
This brings me to a side rant. Seriously, if you're going to post sample code, SHOW HOW TO HANDLE ALL CONDITIONS! In the real world, windows resize. Users hit the wrong buttons. Cameras get unplugged. We need to know how to handle these things! There are still things that the iChat team can do with the iSight camera that I cannot replicate, and I think that stinks.
In summary: here's a situation where I'm actually INVITING you to flame me and post a better way of doing things. Considering the response on my last couple "pimp" articles, where I specifically told people not to, I'm expecting an avalanche of fun on this one.
Labels: code