Saturday, November 03, 2007

Less code vs. good code

Alex Netkachov, Vidyut Luther, and Richard Heyes discuss the pros and cons of writing code that is short. Here are some thoughts from me.

In general, I don't think turning three lines of code into one line makes an application better, or more readable, or prettier, or whatever is your goal. The logical extension of this is to switch to writing Perl code:


#!/usr/bin/perl
# Valentine.pl, copyright 2001-2007 Bill Karwin
map( ( $r=$_, map( ( $y=$r-$_/3, $l[24-$r].=(' ','@')[$y**2-20*$y+($_**2)/3<0] ), (0..30) ), ), (0..24) );
print join("\n", map(reverse($_).$_, @l)), "\n";


The problem with such super-compressed code is that it becomes harder to code, harder to debug problems, and harder to maintain. Imagine being a newly hired developer assigned to a project that contains code all written like the above.

Of course, we're not talking about code as obfuscated as the above. But taking a step in the direction of compressed code for the sake of compressed code, instead of for some functional improvement, is a step toward making the code less maintainable.

For example, think about reading diffs in your revision control system, when a bug fix occurs on such a compressed line of code. It's harder to see the nature of the bug fix, compared to a more expanded form of coding. For this reason, I prefer to write expanded code--even when writing Perl code!

The ternary operator is another good example. Vidyut expresses that the ternary operator should not exist. It's true that it can be abused like any programming construct, but it is definitely useful to have the ternary operator in the language. It is a simple if/else construct, which has a return value. The conventional if/else construct doesn't have a return value. Alex gives an example that is a perfect use of the ternary operator.


$message = ($age < 16) ? 'Welcome!' : 'You are too old!';


But notice I said simple. The ternary operator is inappropriate to use for any if/else blocks that contain anything more complex than a single expression. What if the application requires that we log the event when someone who is too old attempts to access the system?

One can try to do acrobatics in code to accomplish two steps in one expression, but this is not a good practice because if a bug occurs in either step or the requirements change, you end up breaking both.


// Wrong
$message = ($age < 16) ? 'Welcome!' : log_and_return_message('You are too old!');


Once either the positive or negative block becomes anything other than a single expression, you do need to change to using an if/else construct. So if you are tempted to use a ternary operator because it's shorter or it's prettier, consider if there is any likely scenario in which you would have to restructure it anyway. If so, that's probably a good reason to use if/else instead of a ternary expression.

1 comment:

allain said...

I couldn't agree more. The point, I think, is that source code has to be written for human consumption first and then the computer's second.

Except in very special cases, modifying code to make it more human readable is the way to go.