If you’ve been a programmer for any amount of time you’ve more than likely had the honor of inheriting someone else’s code. This might be in a corporate scenario, or you might just be modifying an open source program. Either way you’re experiences though varied are probably marked with few comments, poor syntax, and obscure methods. This might be something you can tolerate, but personally, I am a code perfectionist. I find it difficult to code in new functionality or modify existing functionality without reworking the code to suit my tastes. This can easily go from moving a few braces around to doing a full overhaul.
Unfortunately I can’t offer much advice to those who are stuck in a situation where they are digging through terrible code; honestly all you can do is be patient and buy a stress ball. I can, however, offer some tips on good coding practice that might prevent you from ruining someone’s day.
Tabs and Braces and Spaces
Also known as whitespace. There are few things more frustrating than staring at code that isn’t organized. The number one thing that deters me from helping someone with a programming question is looking at their code and seeing things like this:
$x = 0;
while($x < 10)
{
if($x==3){echo 'This is terrible code';}
else
{
echo 'hello';
}
x++
}
To begin with, the only time braces should not be on a line of their own is when they are preceded, or followed by a conditional statement:
//OK... this method id my personal preference
if($x == 1)
{
echo 'hello';
}
//OK.. probably the most common
if($x == 1) {
echo 'hello';
/*
.
.
.
*/
}
else {
echo 'goodbye';
/*
.
.
.
*/
}
//OK.. I'm not a fan but its acceptable
if($x == 1) {
echo 'hello';
/*
.
.
.
*/
} else
echo 'goodbye';
/*
.
.
.
*/
}
//NOT OK!
if($x == 1) { echo 'hello'; /* . . . */ }
Moving on, code nested within braces or within the body of a conditional or other control flow statement should be indented:
//OK
for($i = 0; $i < 10; $i++)
{
echo "hello \n";
echo $i;
}
/**
* OK,
* Some people don't like this method, but as long
* as it is only a single line its fine with me. Make
* sure you leave an empty space following the
* final line.
**/
if($x == 1)
echo '$x = 1';
else
echo '$x != 1';
//NOT OK
while($x < 10)
{
echo $x;
$x++;
}
Now we come to spaces. Operators and their operands should always be separated by a space. UNIX shell scripters can make an exception to this of course, but otherwise you should space things out:
//All of these are OK
$x = 10;
$y = 5;
$z = $x + $y
$z *= 2;
//These are not OK
$x=10;
$y=5;
$z=$x+$y++;
$z*=($z+$x)--; //I can't even tell you with this is equal to
Duplicate Functionality
The only thing worse than editing your terrible code, is doing it twice. If you find yourself using copy and paste, or otherwise coding the same functionality multiple times, for the sake of anyone reading your code please stop. Not only is this going to translate into a bad experience when it comes time to update your code, but its also likely that whatever mistakes you made the first time you wrote the code were duplicated.
Yesterday I installed a WordPress plugin. The plugin performed well, and for the most part I was happy with it. Of course, however, there were modifications I needed to make (including as it turns out rewriting the code to make it conform to the XHTML standard). Once I opened the file to make the changed I was horrified to discover that there were literally zero functions or objects. If something needed to be done twice, the code was copied. Unfortunately the writer of the plugin forgot to close a div, which is an honest mistake. What isn’t an honest mistake, and should be punishable by death by CRT monitor thrown at you, is taking that code, and copying it three times (error included) to perform the exact same task.
Manageability
That same file I just mentioned contained over 5000 lines of code. I don’t care how large of a project it is, your code should never exceed 1000 lines. Of course, in this case, the programmer decided not to use any functions so there was no logical way to break up the file. If you, on the other hand, are not out of your mind, any significant amount of code you create will contain many functions. Most likely those functions can be categorized and placed in separate files. If you want to get fancy you can even use some objects. The important thing is that the code you write isn’t in blob form.
The same concept applies to individual lines of code. Once again refer to the plugin I had the displeasure of modifying, the longest line in that file was 549 characters long. If I can’t see an entire line of code with scrolling on a widescreen monitor then there is a problem. Really, even using a fullscreen monitor you should never have to use the horizontal scroll. There is nothing wrong with using the return key in the middle of a line of code.
//NOT OK
$animalCount = array('cat' => 5, 'dog' => 2, 'bird' => 7, 'mouse' => 3, 'badger' => 0, /* we don't need no stinking badgers */ 'chicken' => 4);
//OK
$animalCount = array(
'cat' => 5,
'dog' => 2,
'bird' => 7,
'mouse' => 3,
'badger' => 0, //we don't need no stinking badgers
'chicken' => 4 );
Comments
The final portion of my rant regards commenting your code. This is something we’ve all been guilty of at some point. I don’t care how well written your code is, most likely I can’t tell what it does at first glance. If you define a function, write a few words about what that function does. If you do something that seems out of the ordinary, at the very least write down that you did that intentionally.
I wrote some code a few days ago and it contained a switch statement. I intentionally did not use a break in one of the cases because the following case needed to be performed as well. To someone glancing at the code, however, this would seem like a bug, they would add a break statement and introduce a new bug when they thought they were fixing a bug.
Use comments!