Upward Mobility: Overly Defensive Programmer

Sometimes, the best defense is a good offensive core dump

By now, most meme-aware internet surfers have encountered Overly Attached Girlfriend  (and the Rule 63 counterpart, Overly Attached Boyfriend.) What isn’t as well known is that they have a brother, Overly Defensive Programmer (ODP, for short.)

ODP is a mobile developer who lives in the constant fear that the server-side folks are going to subtly change an API out from under him, making his app crash. To avoid this, he puts in code like this:

NSError *error;
NSDictionary *responseData =
     [NSJSONSerialization JSONObjectWithData:data 
                      options:0 error:&error];
if (error != NULL) {
   // All sorts of logging code, followed by returning 
   // from the call silently, swallowing the error
}
if ((responseData[@"payload"] != NULL) &&
    (responseData[@"payload"][@"objectId"] != NULL)) {
     self.objectId = 
       ((NSNumber *) responseData[@"payload"][@"objectId"].intValue;
}

We’ve all seen code like this. The developer didn’t want to accidentally cause a fatal error by trying to get the objectId parameter if the payload was missing, or to try accessing the objectId if it was missing. And if objectId is an optional field, this is totally the way to go.

The thing is, objectId sounds like it is probably a critical piece of data, without which the application will fail to operate properly. You want the application to crash if it goes missing, because that means it will crash during QA testing, rather than silently ignoring the problem and possibly malfunctioning in a way that QA misses.

If you want to be very thorough, you could put in the tests, log what went wrong, and then throw an exception to cause the crash. But all of this is a symptom of a laziness that’s crept into the server side of the client-server picture. Rather than adhere to Contract Driven Development, many modern server developers do API by annotation (I’m looking at you, Java folks!) By sprinkling a few golden @s through their code, the APIs are automatically created. It’s My Little Framework: Annotation is Magic.

The huge problem with this approach is that one little change to an underlying business object can totally change the contract, without a conscious intent by the developer. There can suddenly be a new layer of array wrappers, or a name can change without notice.

I am by no means a fan of SOAP, but one thing I liked about the WSDL-first approach is that both the client and server were required to adhere to a manually curated contract document. Of course, this all went to heck when people started generating the WSDL from the server-side implementation.

I recently have been ripping apart code that, rather than access a dictionary by a key, had been coded to loop through all the keys in the dictionary and try to match against known keys (in a case-insensitive manner) and set values accordingly. So, in other words, he had do this:

for (NSString *key in [dict keys]) {
   if ([key lowerCase] isEqualToString:@"objectId"]) {
      self.objectId =
         ((NSNumber *) dict[key].intValue;
   }
   if ([key lowerCase] isEqualToString:@"objectName"]) {
      self.objectName =  dict[key];
   }
}

Rather than this:

self.objectId = ((NSNumber *) dict[@"objectId"].intValue;
self.objectName = dict[@"objectName"];

Why? Because the original developer hadn’t trusted the backend code, and so had wanted to make sure that objectid or OBJECTID or objectId or event ObJeCtId would work if it was sent down from the server. He didn’t trust the contact!

His original code is much more fragile than the simpler implementation. For example, if the API changes unexpectedly and objectId stops coming down, he’ll never catch it, since it just will fail to appear as a value in the loop. Of course, he could check afterwards to make sure all the required values came down, but they what does he do? Log it and fail silently? Throw an exception? What did all that defensive programming gain him?

In short, he created code that was more likely to silently fail in production, rather than having the application hard-fail if the contract changed without warning. It also was 3-4 times longer than it needed to be, with all those if statements, and much harder to read.

Too many client developers are trying to become ODP, and are essentially creating browsers that permissively try to parse potentially corrupt or illegal data. Client applications are not browsers, and shouldn’t try to be permissive in their parsing. They should be contract grammar Nazis, and die in horrible bloody ways if the server-side data starts to come down in inappropriate formats. Don’t enable lazy behavior by server-side developments, hold their feet to the fire!

And, since clients send data back to the server as well, they should give as good as they get. Client code tends to manually construct data payloads, rather than by annotation or introspection, so this doesn’t tend to be as much of an issue. Changes in data payloads usually require conscious intent, going in this direction. But it’s still important to enforce and obey contracts.

In summary, Don’t Be That Guy. Overly Defensive Programmers aren’t helping anyone, they are only creating an illusion of fault tolerance, while potentially turning a development headache into a deployment nightmare. And remember the core mantra of defensive programming: never catch an exception or check for an error condition unless you are prepared to recover from it in an intelligent manner. Otherwise, you’re just fooling yourself.

tags: , ,