The best defense is a good offense

 

My students might be moving their first careful steps in the dangerous lands of programming, but the problems I’m forcing them to face week after week are nothing short than “difficult”, even in code that is seemingly simple and straightforward. This week we were working on a sprite engine in C# (XNA), adding some code to select different sprite animations to play, where an animation is defined as an array of images. The sprite keeps track of the index of the current image being used in the current animation. Pretty straightforward…. crash.

Oh crashes happen, it’s part of the game, and we quickly found out the source of the problem:

Texture2D GetFrame(int index) 
{   
  return frames[index];    
}

An exception, IndexOutOfBound, was being thrown when a new animation is selected: the current index is being maintained by the Sprite class, and it’s not guaranteed that the new animation contains at least the same number of frames of the previous one. GetFrame() can, potentially, receive an index that is out of bound.

Solution: Make sure GetFrame() doesn’t fail if the index is out of bound.

Something like:

Texture2D GetFrame(int index) 
{   
  if (index > frames.Count)     
     index = 0;   
  return frames[index]; 
}

This is a very common approach to the problem that in my opinion is not actually solving it, merely hiding the dirt under carpet of an if guard. Surely, it “fixes” the bug, but at what cost: some added complexity (it’s an if, one more branch and one more unit test to write in an automated tested environment), that hides away the source of the bug; the system can still be in an inconsistent state. We are just trying to keep going in the presence of a blatant fault.

The more stable solution, I explained to the students, is to fix the source of the inconsistency, in this case either having one separate index per animation or, more simply, reset the frame index to zero when a new animation is selected: an animation is guaranteed to always have at least one frame, the code is thus safe, it doesn’t need a check in GetFrame() (one less branch) and it won’t crash… ever.

Unless we have an animation with no frames.

This is precisely the behavior I want: if there’s an animation with no frames, I want the application to crash and tell me if it’s in an inconsistent state that I can not ignore. If it crashes, I have to fix it.

Fail early, fail loud

This simple example shows my students a broader approach to software construction: offensive programming versus defensive programming. Instead of trying to prevent a crash by writing lots of defensive code that checks anything that can possibly go wrong (but obfuscates meaningful and useful code in a see of ifs), I prefer writing less code, but make it so execution stops as early as possible in the presence of inconsistent state, invalid preconditions, broken invariants.

In my experience this approach leads to less and simpler code, easier to modify and with less crashes.

An obvious drawback of offensive programming might be a live system, that is used by clients (content creators?) while it’s being developed: ring a bell? It’s exactly how we work in the Game Industry. Although I’m convinced that it produces less down time in the long run (by producing better and less crash-y code), it’s hard to convince an artist staring at a call-stack that this is a Good Thing™.

I’m again very open to suggestions here, how would you tackle the problem of writing lean and offensive code versus making the content creators experience pleasant in the presence of a (undoubtedly more rare) show stopper?

Some ideas:

• User friendly crash handler
  
• Automated unit, functional and smoke testing before release to minimize crashes
  
• Build staging

I played a lot in the past with unit and functional testing, especially in the form of asset testing to make sure that assets are sanitized before entering the system, with very promising results that led to fairly stable daily release cycles; something very much appreciated by content creators. Still that show stopper is hard to sell…

This post is also available on AltDevBlogADay.