Friday, July 17, 2009

to block or not to block

"Best practices" documents tell me to always follow an if, while or for statements with a block of code. They suggest I should type this...


if ( missileHitAlienShip() )
{
score += 100;
}


...and never this...


if ( missileHitAlienShip() ) score += 100;


I disagree. I find the latter version much easier to read. If you translate the two versions into English, you get "if a missile hits the alien ship, do the following: raise the player's score by 100" and "if a missile hits the alien ship, raise the player's score by 100." I prefer the second version.

When I present them by themselves, the two versions don't look that different, and the block version isn't all that difficult to read. But when a bunch of blocks are nested inside one another, The code quickly gets difficult to read. Conditional logic gets all mixed up with nitty-gritty implementation details. Compare...


function addToScores() : void
{
if ( levelNumber > 10 )
{
bonus += 500;
}

for ( var i : int = 0; i < numPlayers; i++ )
{
player[ i ].score += bonus;
}
}


... with ...


function addToScores() : void
{
if ( levelNumber > 10 ) bonus += 500;

for ( var i : int = 0; i < numPlayers; i++ )
player[ i ].score += bonus;
}


The "best practices" docs say that you should use blocks because they allow you to easily switch from a single statement to multiple statements. For instance, if I start with something like this...


function addNewAlienShip() : void
{
var ship : AlienShip = new AllienShip();

if ( shipsHaveWarpDrive ) ship.x = Math.random() * universe.width;
else ship.x = alienBase.x;
}


... and later realize I need to also set y-coordinates, I'll need to change my code to something like this:


function addNewAlienShip() : void
{
var ship : AlienShip = new AllienShip();

if ( shipsHaveWarpDrive )
{
ship.x = Math.random() * universe.width;
ship.y = Math.random() * universe.height;
}
else
{
ship.x = alienBase.x;
ship.y = alienBase.y;
}
}


Since I never know how many statements I'll have to add after an if, I might as well start with a block -- as a sort of template -- even if I initially only intend to follow the if with one statement. Or so the "best practices" logic goes.

I disagree. I'm not a fan of blocks, because they encourage multiple levels of nesting and make methods overly long and hard to read. Given the example above, which may be my initial draft, I'd refactor it as...


function addNewAlienShip() : void
{
var ship : AlienShip = new AllienShip();

if ( shipsHaveWarpDrive ) putInRandomLocation( ship );
else putInBase( ship );
}

function putInRandomLocation( ship : AlienShip ) : void
{
ship.x = Math.random() * universe.width;
ship.y = Math.random() * universe.height;
}

function putInBase( ship: AlienShip ) : void
{
ship.x = alienBase.x;
ship.y = alienBase.y;
}


This is easier to read and modular. If I ever want to change the way warp drives place ships, I'll have an easy time doing so without searching through levels of nesting.

I am not dogmatic about forbidding blocks. If a block seems like a simple and clean solution, I use it. But I initially follow my ifs, whiles and fors with a single statement. If I later realize that I need to add more statements, I make a decision. I sometimes replace the single statement with a block. More often, I turn the single statement into a function call, as I did in the example above.

A couple of years ago, I turned a major corner in my life as a coder: I started taking three maxims seriously:

1) each function should do one thing.
2) functions should be as short as possible.
3) nested block are evil.

Previously, I'd given lip-service to these ideas, but when I started following them religiously, my code got way easier to read and maintain.

I have found simplicity on the method level to be more important than simplicity on the class level. Since OOP texts mostly focus on classes, many programmers pay more attention to them than to methods. That's like paying more attention to chapters than to sentences. Good writers know that the sentence is where the real work happens. A well organized table of contents is worthless if each chapter is full of non-sensical sentences.

Again, I am not dogmatic about my three rules, and I often can't follow them when making an initial draft. But when I refactor, I take a close look at any method that is longer than five or six statements. And I take an even closer look at blocks. My goal is to write a group of simple, short functions that each works as one little cog in my big machine.

No comments:

Post a Comment