The random eclecticisms of loonytoons

Coding standards

First off let me apologise if this turns into a little bit of a rant from me but this is a pet topic of mine so I could be liable to¬† go off on one. I’ve also just been working on a project that has had me running into the corner of the room screaming because of the standard of the code (luckily my colleagues are used to my slight idiosyncrasies).

Coding standards are not a new thing by any means, and in my view are really essential to any organisation. They help to promote consistency and ensure that anyone working on a project knows how they should layout and structure their code. This way it becomes much easier to read (especially at first glance), new developers have a handy reference detailing what they should be doing and everyone is generally on the same page with how things should be done.

PHP_codesniffer

Pear LogoTo make life even easier there are applications around like PHP_CodeSniffer that can automatically check that your code confirms to the set coding standards so you don’t have to worry about remembering every single point in the standards. I’m not going to write much about this in detail as there are already some very good references out there such as this one on techportal by lornajane. The documentation on the PEAR site is also very good and shows you how to create your own sniffs, and there are also plugins out that link into some of the most widely used IDE’s, such as this one for eclipse.

Having documented coding standards that are backed up by the use of phpCodeSniffer is also particularly useful in agency or consulting environments where you are often working with 3rd party code bases that each use differing standards. It’s hard enough to try and remember one set of standards, let alone a number of different ones that you are constantly switching in between, so let an application relieve some of the pain by getting it to check your code for you.

Beautiful Code

I’ll be honest and admit that I’m in danger of becoming a bit of a ‘beautiful code’ evangelist (yes, I did just use the word evangelist). I like my code simple, efficient, consistent in it’s structure and layout, and… well, beautiful!

It's not beautiful because it's pink you know...

It’s not beautiful just because it’s pink you know…

Which is why I’ve recently been implementing some coding standards at work, mostly based of the PEAR standards but with a few tweaks that will suit us better. Luckily most of our code at work is pretty good, the majority of the changes required are really just to placate my more retentive side such as to make sure all the spacing is exactly right.

I can’t help it, I really hate looking at code that is badly indented, inconsistent in it’s layout, has redundant code blocks and so on. If I see it I need to fix it! This might be linked to my inability to not adjust pictures that don’t hang straight….

Anyway, this is why one of my latest projects where we are amending some unfinshed code external code is proving quite so painful, as there is not just a lack of any kind of consistency or coding standards but I really do question whether the people writing it had any real knowledge of php at all! If it wasn’t such a large application with a tight deadline then we would have metaphorically ripped it up and started again from scratch but unfortunately we can’t actually do that.

But at least it has provided some excellent examples to demonstrate why I’m not such a loony when I bang on about coding standards all the time. Take a look at the code below, and bear in mind that this is just a short snippet from quite a long page (layout and tabbing is intentional):

	if (mysql_num_rows($result) > 0) {
		echo("<ul class=\"list\">");
    	while($row = mysql_fetch_assoc($result)) {
			extract($row);
	?>
         <li><div class="thumbnail"><a href="list.php?id=<?php echo $id; ?>"><img src="<?php echo $image; ?>" alt="<?php echo $title; ?>" width="50" height="60" border="0"></a></div>

    <h3><a href="list.php?id=<?php echo $id; ?>"><?php echo $title; ?></a></h3>
	</li>
      <?php
		} echo("</ul>");}// end while
		else {

		}
  • Is this easy to read? No.
  • Does it actually make much sense? No.
  • Why are the php tags constantly opened and closed?
  • Why are the echo statements inconsistent (one uses brackets, another doesn’t)?
  • Why does the “end while” comment actually come after the the end of the if statement instead of the while loop?
  • Why have an empty else statement?

So many questions arise from such small amount of script. And when you consider that all the scripts are like this you can see why it would take ages to make any amendments, or for someone new to the project to get up to speed on just what is going on!

And if you want to see something even more scary then check this out:

if (isset($_POST["username"])) {
		$user = $_POST["username"];
	}else{
		echo "you must enter your username and password";
		exit;
	}
	if (isset($_POST["password"])) {
		$pass = $_POST["password"];
	}else{
		echo "you must enter your username and password";
		exit;
	}

	$sql = "SELECT * FROM users WHERE username = \"$user\"";
	$result = mysql_query($sql);
	if (mysql_num_rows($result) != 0) {
		$row = mysql_fetch_assoc($result);
		extract($row);
			if ($password == $pass) {
				session_start();
				$_SESSION["auth_username"] = $user;
				header("location: $ROOTURL/admin/index.php");
			}else{
				echo "Login Incorrect";
				exit;
			}
	}else{
		echo "Login Incorrect";
		exit;
	}

So what lovely things can we see here?

  • The most scary thing that I can see straight away is that if the post variable “user” is set, it is passed into the database straight away without any kind of validation or security at all! This is just a massive problem waiting to happen and just looking at it makes me come over all pale and odd (well, odder than normal anyway).
  • It also makes the ‘interesting’ and seemingly random use of the exit command seem not worth worrying about.
  • Other than that though the layout of the code is almost normal!

Part of the problem here does go beyond my topic of coding standards and reaches into the worlds of code security, language familiarity, and developer training and support (which I won’t cover here as I’ll never end). But coding standards can play a big part in avoiding a lot of the issues shown above and can help to ensure that everyone learns and follows best practices which can only be a good thing.

In Conclusion

This hasn’t been just an opportunity to showcase some really shocking code that you can all gasp over.

Everyone makes mistakes, nobody is perfect. I can guarantee that my code is in no way perfect (and this is not an invitation to pick it apart either), but I can assure you that I work extremely hard to constantly try to learn and improve how I work so that maybe someday, however unlikely, someone might look at my code and think “wow, this girl is truly a masterful genius, I must give her all my money at once”. I would settle for just a smile and a compliment though. And maybe a beer or two…

However coding standards do help to promote a consistent and efficient approach that results in more readable and amendable code, and I think that these examples go a long way towards proving just how much of a benefit that would be.

I’m just hoping to inspire people to realise that there is a point to beautiful, well laid out and efficient code, beyond the fact that it fulfills the function it was designed to undertake.

Thankyou.

</rant>

Share

3 thoughts on “Coding standards

  1. LornaJane says:

    Nice rant :) I think you have such a valid point about readability and the code samples were absolutely cringeworthy!

  2. loonytoons says:

    @LornaJane Thanks! The scary thing is that those are real code samples – I have honestly not made them up to prove a point!

  3. Jennifer (littleoracle) says:

    awesome rant! no matter the language, clean, beautiful code is so important! it makes me itchy to see stuff like that and i always wonder how the developer even knew what was going on.

Leave a Reply

Your email address will not be published. Required fields are marked *