Introduction
Producing code that clearly conveys a developer’s intent is key to any well written application.
That not only applies to PHP, but every programming language. Developers
who emphasize the creation of legible code tend to create applications which are easier to
both maintain and expand upon. After seven years of programming in PHP I’ve worked on a variety of
projects where well organized and legible code were set aside for numerous reasons. Some of those
reasons include time constraints, lack of experience, lost enthusiasm, misdirected pre-optimizing,
and the list goes on.
Today we’ll look at three simple methods which are commonly ignored by developers for some,
if not all of the reasons described above. First, we’ll discuss the importance of clean conditional
logic. Second, we’ll look at how you can cleanly output blocks html of in PHP. And finally, we’ll
examine the use of sprintf to convey variables placed in stings more legibally.
Tip #1: Write Clean Logic Statements
Example 1.1: Unclean Conditional Logic
<?php
if($userLoggedIn) {
// Hundreds of lines of code
}else{
exit();
}
?>
The above statement seems straight forward, but it’s flawed for the reason that the developer
is giving this conditional block too much responsibility. I know that might sound a little weird,
but stay with me.
The type of conditional organization above makes for unnecessarily complex code to
both interpret and maintain. A brace that’s paired with a control structure
hundreds of lines above it won’t always be intuitive for developers to locate.
I prefer the style of conditional logic in example 1.2, which inversely solves the previous example.
Let’s take a look.
Example 1.2: Clean Conditional Logic
<?php
if(!$userLoggedIn) {
exit();
}
// Hundreds of lines of code
?>
This conditional statement is more concise and easier to understand.
Instead of stating: “if my condition is met, perform hundreds of operations, else exit the script”,
it’s saying “if my condition is not met, exit the script. Otherwise, I don’t care about what
happens after that. I am only concerned with stopping execution”. So, by doing this,
you’ve limited the operations that a given control structure has been tasked with, and that will help other
developers quickly understand your code.
Tip #2: Use Less PHP And More HTML With Alternative Syntax
When PHP is tightly intertwined with HTML it can be awful to lay your eyes upon.
Many years ago, while with a previous employer, the development staff and I believed it was a
sin to use straight HTML in PHP files. We believed every file ending in a .php
extension could only contain PHP. In retrospect, I don’t know why we ever did this,
but I have seen other groups adhere to such rules as well. This style made what should have been simple HTML far more complex that it had to be.
Over the past few years the style in which
developers are writing PHP to output HTML has shifted quite a bit. Let’s look at Example 2.1
Example 2.1: No Alternative Syntax, and a Lot of PHP.
<?php
echo "<table size=\"100\">\n";
echo " <tbody>\n";
if($displayResults) {
while($row = mysql_fetch_assoc($result)) { ?>
echo "<tr>\n";
echo "<td>" . htmlentities($row['id']) . "</td>\n";
echo "</tr>\n";
}
}
echo "</tbody>\n";
echo "</table>\n";
?>
This is typical to what I’ve seen in a number of PHP applications.
Though this is a simple example, I am sure you can appreciate how this
method of outputting mostly html can grow out of control. It requires
significantly more developer brain power to determine the code’s
purpose than the example below.
Example 2.2: Less PHP Paired With Alternative Syntax
<table>
<tbody>
<?php if($displayResults): ?>
<?php while($row = mysql_fetch_assoc($result)): ?>
<tr>
<td><?php echo htmlentities($row['id']); ?></td>
</tr>
<?php endwhile; ?>
<?php endif; ?>
</tbody>
</table>
Here in example 2.2 the same functionality is achieved by using PHP alternate syntax and
less output statements. As you can see, this fashion of interweaving PHP and
HTML can go a long way to increase code readability.
Tip #3: Use “sprintf” and Friends
Example 3.1: Uncleanly Spliced String.
<?php $sql = "SELECT col1, col2, col3 FROM people WHERE first_name = '" . mysql_real_escape_string($first_name) . "' AND last_name = '" . mysql_real_escape_string($last_name) . "'AND foo = '" . ($bar = "good" ? "good" : "bad") . "' ORDER BY col1" ; ?>
I see these types of indecipherable strings –like this SQL
statement example– all too frequently. The meaning of this query
has been lost due to numerous concatenations and escape
functions; which means developers have to invest a significant
amount of time to comprehend the code.
In order to avoid this this, I use PHP’s sprintf() function. sprintf() is a
function that’s part of a family of functions –referred to as the “printf” family of function–
that substitute designated tokens with arguments to the function.
For example, let’s look at the code in example 3.2.
Example 3.2: Cleanly Assembled String With sprintf()
<?php
$sql = 'SELECT col1, col2, col3 ' .
'FROM people ' .
'WHERE first_name = "%s" ' .
'AND last_name = "%s" ' .
'AND foo = "%s" ' .
'ORDER BY col1 ';
$sql = sprintf($sql, mysql_real_escape_string($first_name),
mysql_real_escape_string($last_name),
($bar = "good" ? "good" : "bad"));
?>
This method allows developers to regain a sense of the data that
they’re representing. In this particular example the %s token means replace
with a string. There a number tokens such as %d (decimal) and %f (floating point).
Check out PHP’s sprintf manual page
for a more detailed account of what you can do with the “printf” family of functions.
Conclusion
I hope this article has provided some information you find useful.
Making your PHP easy to understand by others
should be a top priority for every developer, and I believe that these three simple tips can serve as a start on that journey.
June 24, 2008 at 2:09 pm
First of all, great article! I’m glad to see these best-practices articles posted here. If only more people would write readable code
I know your example was about cleanly assembling strings, not about creating database queries. However, I just want to point out that one should really used PDO <http://www.php.net/pdo> or Zend_Db <http://framework.zend.com/manual/en/zend.db.html> to either quote identifiers and values (instead of using mysql_real_escape_string) and/or create parameterized prepared statements (this would be the preferred method).
June 24, 2008 at 2:12 pm
Point 1 is reasonable for readability, but generally, calling exit() in the middle of a script is clumsy. You almost always want to have some code to handle the error, exception or mistake.
Point 2. Please, no. Years of work have gone into the MVC principle and the separation of logic and presentation code, and while neither of these code examples are good, the first is at least readable as code and recognisable as PHP.
Ideally, don’t mix HTML and PHP. If you must do, put the emphasis on code flow, not on HTML prettiness.
Point 3. Possibly the least alarming, although the preference would be to use a decent DB abstraction layer, ideally with prepared statements.
Yes, break your lines, but if you *must* build SQL by raw string construction, you can either pre-escape your variables and concatenate the ‘readable’ version, or just escape in-line.
For genuinely readable code, take a look at the Zend Coding Standard: http://framework.zend.com/manual/en/coding-standard.coding-style.html
June 24, 2008 at 3:02 pm
Point 1:
These examples are simple little snippets that have no real world use. Sure, exit() in the real world is a poor way to control the flow of an application, but come on, this is just a simple example.
Point 2:
Not all projects you encounter will be MVC. Don’t get me wrong, I love MVC. It’s my preferred method of writing apps, however there are a number of developers who don’t use it. And, you may just find yourself working on one of their projects.
Point 3.
Sure, you should use a DB abstraction layer. I personally use Zend_Db and PDO. I guess it could have been helpful to point that out. That said, sprintf() has farther reaching uses than just SQL. Any string were you don’t want to perform a number of concatenations, or want specific formatting would would be an apt use.
-Nick
June 24, 2008 at 3:13 pm
wechsler I think you kind of missed the point. These examples were simply to express a point, I’m sure the author knows about the best practices you pointed out, there just isn’t room for that in these examples.
June 24, 2008 at 3:59 pm
Anyone who has working experience with MVC will realize the true nature of separating a presentation layer from the application. It isn’t naively keeping PHP away from HTML– to be quite frank, PHP is an excellent templating language, and will always be used with HTML in simple forms, such as the examples above.
The practice of separating the presentation layer is keeping the interface of the application away from the business side of the application, to allow for easy changing and fewer dependencies. Had the author included vital application logic (included system libraries, checked user auth, etc) within the examples above, this wouldn’t be following MVC standards.
Off-topic: don’t knock sprintf; the idea of sprintf is extremely useful and less error-prone. I would *much* rather have a SQL statement constructed with sprintf than with just concatenation.
June 24, 2008 at 6:59 pm
I understand that you weren’t promoting the use of exit() in a program, but I think the first example violates having a single exit point, and complicates the code as well. Rather than halting before getting to a certain block of code, I’d suggest branching into contained blocks of code. If you ever have hundreds of lines of code in an if statement, consider refactoring that into a method. In my opinion, the code below is more readable and maintanable than what was given in example 1:
if($userLoggedIn) {
$this->handleLoggedIn();
}else{
$this->handleNotLoggedIn();
}
Its easy to read whats happening, and when, and if the methods need to increase in size, thats fine, this portion that branches is always very easy to follow. Just my 2 cents.
June 25, 2008 at 7:45 am
Hi, I made this small extension to the PHP language to make it more readable.
You can write code like
$test = .string lowercase("This IS UPPERCASE");
The idea is to make the PHP functions more human readable, also I integrate Zend Framework with Doctrine and JQuery.
<a href="http://www.orsa-studio.com/phptaco/">http://www.orsa-studio.com/phptaco/</a>
http://www.orsa-studio.com/phptaco/
June 25, 2008 at 4:35 pm
It’s not often that I disagree with a DevZone article.
Point 1: Absolutely correct. Especially the "Bail on Failure" rule of thumb of the second example. This can save lots of time when scanning code at a glance.
Point 2: Nope. If you’re after legibility, not only have you just added lots of visual complexity around each and every snippet of PHP code the user is trying to locate and identify at a glance, you have ignored one of the basic helpful capabilities of every single text editor and IDE out there: Syntax coloring. Every editor a developer is likely to use will color code both HTML and PHP, very often using the same default reds, blues and greens. But PHP and HTML colorization uses each of those colors for slightly different purposes, and mixing your code with alternate syntax guarantees that the first question the developer will have to ask themselves when looking at any particular piece of text is "Is this PHP or is this HTML?".
By assembling your HTML as strings, you colorize all HTML elements with a single color, making it cognitively very easy to separate HTML from PHP code at a glance. This mishmash of HTML and PHP structures is precisely what creates the impression of spaghetti code that so many outsiders deride PHP for. There are certainly good times for the practice, but don’t force the developer to change mental gears between HTML and PHP unless there’s a good reason. If you want legibility, use the separation of HTML and PHP to visibly reinforce the structure of your PHP logic, not the structure of your HTML.
Point 3: Sprintf is fabulous and can let you do lots of good things, but again, if you want legibility, sprintf separates one of the simplest of PHP processes (concatenation) into a far more complex templating process. Practice will teach you how to comprehend it at a glance, but its not simpler and its not more legible.
June 25, 2008 at 5:50 pm
Great points in the article. A good real world case for point 1 is when you want to return inside of a function when certain checks fail.
<?php
function test()
{
if ($check1Failed) {
return false;
}
if ($check2Failed) {
return false;
}
// lots of code to return a legit value
return $goodValue;
}
?>
It really helps with the code readability with returns inside of functions.
June 29, 2008 at 9:57 pm
The first point uses negative logic that could be avoided. As has been covered, you’re not generally going to exit a PHP script like that, you’re more likely to provide something useful to the user. Given that, there is no need for negative logic in the example, it would be far better to have something like Brad Harris suggested. Negative logic does have its place, but not in the example provided.
I also have to disagree with point 2. There are pluses and minuses to both, but do you want to be reading PHP or HTML or both at the same time? I’d rather deal with the "bad" example than the "good" example because, basically, it is far easier to miss the programmatic parts in the "good" example. However, neither are really great examples of the best approach.
The problem with this tutorial isn’t the points, actually, it’s the examples. The points have some validity (though point 2 is debatable), but the chosen examples make many of us ask "what?!?" in the process. In the first point, I would have used the Brad Harris example. For the third point, you would have been better off with something like a translation example since nobody thinking about database interaction intelligently would actually do your examples.
July 1, 2008 at 7:50 am
Good read,
About #3: I was wondering what the general opinion is on heredocs, as those are great for constructing good readable SQL.
On the other hand, with good practices like templating and db-abstraction, constructing these kind of text-blocks will probably becomes less important…
July 10, 2008 at 2:56 am
Hey! i am new in php programming language,everyone who really knows php can answer my question.
How can i manipulate the input that comes from the user?Please i want to learn more about php. thanks and godbless
July 21, 2008 at 10:02 pm
NEVER EVER use sprintf(). Its evil.
Reason 1: Code may "look" cleaner, but its not (though I don’t see what’s unclean or unreadable about normal concatenation). Its a nightmare to edit and maintain after its been written, especially if you’re using much longer queries or text with more replacements. Then if you accidentally take out a %s, the order of the replacements is out of order and its just no fun at all. It gets hard to keep track of everything.
Reason 2: Its **TWICE AS SLOW** as normal string concatenation.
If you need to "clean your code", use this method (also gets rid of ternary operators, yuk!):
$sql = ‘SELECT col1, col2, col3 FROM people WHERE ‘;
$sql .= ‘first_name = "’. mysql_real_escape_string($first_name) . ‘" ‘;
$sql .= ‘AND last_name = "’. mysql_real_escape_string($last_name) . ‘" ‘;
if($bar == "good"){
$sql .= ‘foo = "good" ‘;
}else{
$sql .= ‘foo = "bad" ‘;
}
$sql .= "ORDER BY col1";
Now that’s nice, clean, readable, editable AND fast!
August 6, 2008 at 12:23 pm
I like HEREDOC for HTML blocks, personally I usually prepare all the content I may need in PHP variables and then use a HEREDOC almost like a template only placing the PHP variable names within the HEREDOC to output the data I want.
I find HEREDOC much neater than switching in and out of the PHP context or using echo’s with concatenation, in fact I said so on my blog at www phpzone co uk
You have to be careful you place the HEREDOC start/end markers correctly, but other than that its much nicer to see, and yes I do sometimes use them for SQL statements too.
In short, if your not using a templating system, I do recommend HEREDOC over any other choices.
August 7, 2008 at 7:19 pm
I like to use Tip #2 because it adds flexability in changing the output whenever you need to.
For example:
<?
…other code
while ($row = mysql_fetch_assoc($query))
{
echo "<tr><td>";
echo $row["name"];
echo "</td><td>";
echo "$row["date"];
echo "</td></tr>";
echo "<tr><td colspan=\"2\">";
echo $row["comments"];
echo "</td></tr>";
}
?>
That’s kind messy and when you are trying to debug a page by looking at the source code through the browser, it may not be visually aligned right. Also, maybe you want to add a new field or design the structure differently, so you’ll have to add more echo’s and more double-quotes and more escaped-double-quotes and semi-colons, etc.
<?
while ($row = mysql_fetch_assoc($query))
{
?>
<tr>
<td>By: <?=$row['name'];?> on <?=$row['date'];?>
</td>
</tr>
<tr>
<td><?=$row['comments'];?></td>
</tr>
<?
}
?>
Any changes needed to be made (big or small) within the HTML display can be made easily. These were simple examples since they only dealt with rows instead of a whole table.
<?
while ($user = mysql_fetch_assoc($user_query))
{
echo "<table border=\"0\">";
echo "<tr><td>Username:</td><td>" . $user['username'] . "</td></tr>";
echo "<tr><td>First Name:</td><td>" . $user['first_name'] . "</td></tr>";
echo "<tr><td>Favorite Food(s):</td><td>". $user['foods'] . "</td></tr>";
echo "</table>";
}
?>
By the end of all that, you’ll have carpel-tunnel (if you don’t already)!
<?
while ($user = mysql_fetch_assoc($user_query))
{
?>
<table border="0">
<tr><td>Username:</td><td><?=$user['username'];?></td></tr>
<tr><td>First Name:</td><td><?=$user['first_name'];?></td></tr>
<tr><td>Favorite Food(s):</td><td><?=$user['foods'];?></td></tr>
</table>
<?
}
?>
Much more cleaner and the editors I’ve used color code the html to be distinguished from the php, so analyzing code is quite easy as well as inputting the code.
I may have repeated the basic point of Tip #2, but it’s my first time posting here and I felt like putting a few other points in.
September 8, 2008 at 8:53 pm
i dont like the sprintf version. if you have to change your sql-code you always have to search for the applicable sprintf-part.
i prefer this code style since working with php5:
$obj->myEscapeMethod(array($first_name, $last_name, $bar));
$sql = "SELECT col1, col2, col3
FROM people
WHERE first_name = ‘{$obj->myEscapeMethod($first_name)}’
AND last_name = ‘{$obj->myEscapeMethod($last_name)}’
AND foo = ‘{$bar = "good" ? "good" : "bad")}’
ORDER BY col1";
in my opinion it can’t be simpler and easier to read or change!
myEscapeMethod could be something like:
function myEscapeMethod(string $string, $mysql_identifier = null)
{
return is_null(mysql_identifier) ? mysql_real_escape_string($string) : mysql_real_escape_string($string, $mysql_identifier);
}
April 16, 2009 at 11:34 am
I personally think that writing HTML code inside ‘plain’ PHP strings is more of mixing html and php than using alternative syntax… above all it gets unreadable when you use escape sequences.