Three Points On Error Handling

I’ve been ranting on Twitter lately about how to do error checking the wrong way. Coincidentally, the latest topic on Mac Developer Roundtable was about just that, error handling.

I don’t think this episode of MDR was very interesting. It was essentially an overview of the three common error handling strategies (exceptions, enum returns, pass-by-ref error object), and some general C++/Java likeage from Uli (which is not a compliment :P). However, it got me writing a real blog entry, which is a good thing :)

So: Three things kept repeating in my head while I listened to MDR#3, hoping that someone’d mention it so that we can rid the world of more bad code.

First off, stay far far away from nested ifs. I don’t remember which Mac dev blog I read it on, maybe Wil Shipley’s or ridiculous_fish, but whoever it was recommended to always try to keep the code as far to the left as possible. Use the outermost block for the common, correct case, and use inner blocks for error cases. What I mean by this is, check for the error condition and treat it in-place, don’t check for the not-error case and treat the error in an else far far away. An example is in order…

// Don't:
void collectPhazon() {
  PhazonDetector *detector = findNearestPhazonDetector();
  if(detector) {
    Phazon *nearestPhazon = detector->scanForPhazon();
    if(nearestPhazon) {
      sendPhazonCollectorDrone(nearestPhazon->position());
    } else {
      beep(kNoPhazonFoundBeep);
    }
  } else {
    beep(kMajorlyCatastrophicErrorBeep);
  }
}

// Do:
void collectPhazon() {
  PhazonDetector *detector = findNearestPhazonDetector();
  if(!detector) { beep(kMajorlyCatastrophicErrorBeep); return; }
  
  Phazon *nearestPhazon = detector->scanForPhazon();
  if(!nearestPhazon) { beep(kNoPhazonFoundBeep); return; }
  
  sendPhazonCollectorDrone(nearestPhazon->position());
}

This also applies to smaller scopes, such as for and while loops. Instead of having a big if block inside the loop, say if(error_condition) continue;. Feel free to use gotos as well, eg:

  Baz *a() {
    ...
    if(error) goto a_cleanup;

    return myBaz;
  a_cleanup:
    free(myBaz);
    fclose(bazHandle);
    return NULL;
  }

The same effect can be achieved with try-catch-finally, so use whichever makes your code the easiest to understand.

Secondly, never use just a boolean NO or a nil return value as an error return for a method or function that can fail in more than one way. If you do, the user of your library (or yourself, if it’s in your app!) can’t know what actually went wrong.

This leads to error dialogs such as “Couldn’t connect to iPod” — why not? Because one isn’t connected? Because it’s the wrong model? Because I hit it with a hammer? The sentence is lacking a ‘because’ because the reason is hidden behind bad abstractions.

This is where NSError is your friend, and I think that this is the answer to Scotty’s burning question: ‘When should I use NSError’? The answer is simple: whenver there’s more than one way to fail; whenver a NO/nil can mean more than one kind of failure.

Third, and this was actually mentioned, if you’re checking for errors, make sure you understand /why/ you’re checking for that error, and what the logical response is.

This snippet from Apple’s sample “ComplexPlayThru” is a perfect example of how not to do it (ComplexPlayThru.cpp, line 353-354):

  comp = FindNextComponent(NULL, &desc);
  if (comp == NULL) exit (-1);

Nice! My app suddenly disappeared with no explanation whatsoever! And I’m sure no other part of the app uses the error code ‘-1’! Awesome. Also, the component that they’re looking for is an Apple default, must-be-there-or-any-sound-app-will-crash component. It’s safe to assume that it will always be there, and just skip the error check.

This also goes against Gus’ advice: please don’t just copy any sample code you find, even if it’s from Apple.

NSURLConnection, rails, apache, spaces in URLs and “Your browser sent a request that this server could not understand”

My Rails application uses redirect_to at one point in my code where it redirects to a pdf document on an apache server. The URL that it redirects to contains spaces. URLs may not contain spaces. redirect_to does not escape these spaces, but issues a ‘302 Found’ redirect response with the unescaped URL.

When my Cocoa application receives this redirect request through the currently running NSURLConnection, it follows the new url. In 10.4, it escaped the URL before sending the GET request. In 10.5, this is no longer the case, and apache (correctly) barfs on the request with

Bad Request

Your browser sent a request that this server could not understand.

The request line contained invalid characters following the protocol string.

The solution is simple: URI::escape the URL before redirecting to it. The big question however, is: Is this a bug in my code, in Rails, or in Cocoa?

I’m guessing first or second; I’m supposing there’s a very good reason why Apple chose to change the behaviour of NSURLConnection. Also, the Rails documentation does say, String starting with protocol:// (like http://): Is passed straight through as the target for redirection, which would put the responsibility square on me. However, Rails is generating an invalid response, so I’ll go with blaming rails.

Mr. Random Is My Friend

This is Mr. Random.

21:35:26 nevyn@xephon:~$ cat bin/rnd
  #!/usr/bin/env ruby
  class Array
    def random
      self[Kernel.rand(self.length)]
    end
  end


  if $0 == __FILE__
    puts $*.random
  end

This is how you use Mr. Random.

21:38:38 nevyn@xephon:~$ rnd pizza chinese leftovers
leftovers 

The best thing is how it makes you realize what you don’t want.

21:38:50 nevyn@xephon:~$ rnd pizza
pizza 

Further anecdotes from Java Hate Land

Style is a class that inherits from MutableAttributeSet which conforms to AttributeSet. An AttributeSet contains key-value pairs describing character attributes, which works okay. However, Style contains additional state about character attributes that is seemingly completely separate from the state held by its superclass. Using the interface from AttributeSet to set character attributes yields no result whatsoever — the resulting text is unstyled.

	Style s = new Style();
	s.addAttribute(TextAttribute.UNDERLINE, TextAttribute.UNDERLINE_ON);
	// Rendering text using s yields plain text, no formatting.

To actually set the style of a Style, you use the class StyleConstants’s static methods set[type of attribute](MutableAttributeSet, newValue), like so:

	Style s = new Style();
	StyleConstants.setUnderline(s, true);

Recall that AttributedString does not inherit from String. I’m beginning to believe that the Java designers never realized what object orientation is and just add inheritance to classes where it sounds good.

There seems to be no way whatsoever to represent styled text in Java that is in any way portable across the Java API. AttributedString only works with printing; AttributedCharacterIterator is the only way to get text and style out of the AttributedString; Style is used by the UI to represent style but while it actually inherits from the same class as the internal style representation in AttributedCharacterIterator, it’s actually completely incompatible. I’ve seen even more ways to represent style, but haven’t used them so I can’t recall them at the moment.

Right now, I’m going through hell to be able to get a formatted text representation of a model class of mine, being able to either append it to a JTextPane or send it directly to the printer, and later being able to extract the styled text from the JTextPane and send it to the printer. The text is going through at least three different representations of style on the way.

Please, for the love of god, tell me that I’m missing something blatantly obvious that makes this make any sense whatsoever.

I hate the Java API. Truly. With all my heart.

A JTextPane that shows styled text can’t take an AttributedString. You can’t get the string out of an AttributedString without iterating through it, because it doesn’t inherit from String. You can insert attributed text into a JTextPane, but only by giving it a String and an AttributeSet. You can’t get an AttributeSet out of an AttributedString, but you can get a map out of it. Which are two completely different things, it seems.