Line breaks are friends not food

One of the things that never fails to make me want to repeatedly slam my head into my keyboard when looking at other peoples code is inconsistent or none existent white-space.

There is no excuse for being inconsistent anywhere in any code you write. I say this not just because its the mark of a lazy developer but because it makes the life of the next person to work on the project unimaginably harder. To illustrate the point take a look at this, okay rather extreme, example of terrible indentation

<?php

if ($_FILES['file']['error'] == 0){
		if ($_FILES['file']['size'] > 1024000){
	echo 'File too big :(';
}

else{
if (!move_uploaded_file($_FILES['file']['tmp_name'], '/somewhere/we/store/files.txt')){ echo 'upload failure';

}
}

?>

I know I know, I’m going over the top but I’d bet most developers have seen something like the above at least once. The problem with this is that even though the code is very simple it takes a good number of seconds to figure out because of the wonky use of tabs. Here’s how it _should_ have looked

<?php

if ($_FILES['file']['error'] == 0){
	if ($_FILES['file']['size'] > 1024000){
		echo 'File too big :(';
	}else if (!move_uploaded_file($_FILES['file']['tmp_name'], '/somewhere/we/store/files.txt')){
		echo 'upload failure';
	}
}

?>

Anyone bothering to read this post should find the second block far clearer!

The same motivation that causes us to, hopefully, prefer the second block can be taken too far to the point that we actually end up with a far less readable script. Wanting to do everything in as few characters as possible is almost universal and often leads to a more concise program, taking that too far can almost be as bad as being inconsistent. The most common method that I’ve seen people using to save characters for the sake of it is dropping curly brackets and line breaks. Instead of

if ($username == 'bob'){
	echo '<a href="bob.html" title="Bobs secret page">Secret</a>';
}

you’ll see just one line

if ($username == 'bob') echo '<a href="bob.html" title="Bobs secret page">Secret</a>';

which is nothing too terrible just yet. Combine a few similar looking checks and it all starts to get a bit less clear though

if ($username == 'bob') echo '<a href="bob.html" title="Bobs secret page">Secret</a>';
if ($last_login > time() - 3600) echo '<a href="how_did_i_get_here.html" title="What an odd condition">Strange</a>';
if (user::get_logged_in()->get_username() == $username) echo '<a href="logout.html" title="Logout">Logout</a>';
if (isset($_SESSION['previous_page'])) echo '<a href="', $_SESSION['previous_page'], '" title="Back to previous page">Previous</a>';

Hopefully we’re beginning to see how it would be easy to create a complete mess if you give up on basic rules of indentation, it takes a bit of thought to see which condition leads to which .html page in the block above. Admittedly not a huge amount of thought, but those few seconds do add up not to mention the frustration caused.

The step up from this is the complete removal of pretty much all white-space, the main problem with this is that all of the characters tend to merge together at a glance. This makes it rather too easy to mistake a == for a = in a conditional statement.

if($username='bob'){
	echo 'hi bob!';
}

Combining all of the bad stuff we very quickly end up with something pretty hard to follow like this fictional PDF generating code which is rigidly consistent, but still bloody hard to make sense of

$pdf=new FPDF();
$pdf->AddPage();
$pdf->SetMargin(10,10,10);
$pdf->SetFont('arial','b',16);
$pdf->Cell(0,10,'Full list of website users',0,1);
foreach($users as $user) if ($user['type']==2){
	$pdf->SetFont('arial','b','10');
	$y=$pdf->GetY();
	$pdf->Cell(60,5,'Name:',0,1);
	$pdf->Cell(60,5,'Email:',0,1);
	$pdf->SetXY(70,$y);
	$pdf->SetFont('arial','',10);
	$pdf->SetTextColor($user['admin']?0:60);
	$pdf->Cell(0,5,$user['name'],0,1);
	$pdf->Cell(0,5,$user['email'],0,1);
	$pdf->Ln();
}
$pdf->Ln();
$pdf->Output('','S');

Simple code, but that should take a good few seconds of looking for most people to make sure they get what’s going on. The foreach and if inline are the worst thing by far, surround that with a few long lines and you could spend a long time trying to figure out why $users only contains a handful of the expected list. On top of all of this there is the lack of confidence you get from finding such a ugly bit of code, after seeing that you’ll have to pay very careful attention to the rest of the source to make sure you don’t miss any other disguised conditionals.

So to sum up, for the love of god use some white-space! The few KB you save are really not worth the time it takes to make sense of the mess left over after you trim out all the clarity.