Just like any language, developers can write code in PHP that ranges in quality from truly awful to very good. Learn good programming habits that can help you bridge the productivity gap.
Depending on whom you ask, the difference between a good developer and an excellent developer, in terms of productivity, is a factor of 10 to 20. An excellent developer is more productive because of his experience and good habits. When poor programming habits sneak into your code, they're a drain on productivity. This article demonstrates some good programming habits that can make you a better programmer.
In addition to enabling you to build code more productively, these habits can help you build code sustainable for an application's lifetime. Any code you write is likely to spend most of its lifetime in maintenance; application maintenance is a large expense. Establishing good coding habits will enhance design factors like modularity, and your code will be easier to understand and, thus, easier and cheaper to maintain.
Bad coding habits seem to accompany defects in code and can cause code to be difficult to change without introducing new defects. The following five good habits, when applied to your PHP code, will help you avoid these pitfalls:
- Use good naming.
- Take smaller bites.
- Document your code.
- Handle error conditions.
- Never, ever, copy and paste.
The next sections explain these habits in detail.
Use good naming
Using good naming is the most important habit because descriptive names make code easier to read and understand. The understandability of your code ultimately determines whether it can be maintained in the future. Even if the code you write contains no comments, if it's easy to understand, it will be much easier for you or someone else to change, if necessary. Your aim, when developing this practice, should be to use good naming to make your code read much like a book.
Bad habit: Ambiguous or meaningless names
Listing 1 shows code that includes overly short variable names, abbreviations that are difficult to understand, and method names that don't clearly describe what the methods do. Method names that imply the methods do one thing when they really do something else can be particularly problematic because they're misleading.
Listing 1. Bad: Ambiguous or meaningless names
<?php
function getNBDay($d)
{
switch($d) {
case 5:
case 6:
case 7:
return 1;
default:
return ($d + 1);
}
}
$day = 5;
$nextDay = getNBDay($day);
echo ("Next day is: " . $nextDay . "\n");
?>
|
Good habit: Reflective yet concise names
Listing 2 demonstrates code that uses good naming habits. Methods are renamed to be more reflective of what they do and why. Variables are renamed to be more descriptive, as well. The only variable that's left short is $i, which in this listing is a looping variable. Although many people may disagree, a short name for the looping variable is acceptable — even good — because it's a clear indicator of functionality.
Listing 2. Good: Reflective yet concise names
<?php
define ('MONDAY', 1);
define ('TUESDAY', 2);
define ('WEDNESDAY', 3);
define ('THURSDAY', 4);
define ('FRIDAY', 5);
define ('SATURDAY', 6);
define ('SUNDAY', 7);
/*
*
* @param $dayOfWeek
* @return int Day of week, with 1 being Monday and so on.
*/
function findNextBusinessDay($dayOfWeek)
{
$nextBusinessDay = $dayOfWeek;
switch($dayOfWeek) {
case FRIDAY:
case SATURDAY:
case SUNDAY:
$nextBusinessDay = MONDAY;
break;
default:
$nextBusinessDay += 1;
break;
}
return $nextBusinessDay;
}
$day = FRIDAY;
$nextBusDay = findNextBusinessDay($day);
echo ("Next day is:" . $nextBusDay . "\n");
?>
|
You are encouraged to break up large conditions into a method and name the method so it describes the condition. This technique makes the code easier to read and externalizes the condition so it can be abstracted and perhaps even reused. If the terms of the condition change, it's easier to update the method. Because the method has a meaningful name, the code doesn't lose its meaning or become difficult to read.
 |
|
Take smaller bites
It's easy to get focused on solving a problem and start coding away. While you're solving an immediate problem, you keep typing, letting your functions get longer and longer. That's not a problem over the long term, as long as you go back later and refactor the code into smaller bites.
Refactoring is a great idea, but you should develop the habit of writing shorter, more focused methods the first time. Shorter methods that can be viewed in one window are easier to understand. When a method is too long to be viewed all at once in a window, it decreases in understandability because you can't quickly follow its entire flow from beginning to end.
When building methods, you should also form the habit of constructing them so they do one thing and one thing only. There are several reasons for such diligent focus in your method writing. First, methods are more easily reused when they do one thing and do it well. Second, such methods are easier to test. Third, such methods are easier to understand and change — if necessary — the simpler they are.
Bad habit: Really long functions (that do a lot)
Listing 3 shows a long function. It's problematic for a couple of other reasons. It does many things, so it isn't cohesive. It will be harder to understand, debug, and test. It iterates through a file and builds a list of entries, it assigns the values to objects, it does some calculations, and more.
Listing 3. Bad: Long functions
<?php
function writeRssFeed($user)
{
// Get the DB connection information
// look up the user's preferences...
$link = mysql_connect('mysql_host', 'mysql_user', 'mysql_password')
OR die(mysql_error());
// Query
$perfsQuery = sprintf("SELECT max_stories FROM user_perfs WHERE user= '%s'",
mysql_real_escape_string($user));
$result = mysql_query($query, $link);
$max_stories = 25; // default it to 25;
if ($row = mysql_fetch_assoc($result)) {
$max_stories = $row['max_stories'];
}
// go get my data
$perfsQuery = sprintf("SELECT * FROM stories WHERE post_date = '%s'",
mysql_real_escape_string());
$result = mysql_query($query, $link);
$feed = "<rss version=\"2.0\">" .
"<channel>" .
"<title>My Great Feed</title>" .
"<link>http://www.example.com/feed.xml</link>" .
"<description>The best feed in the world</description>" .
"<language>en-us</language>" .
"<pubDate>Tue, 20 Oct 2008 10:00:00 GMT</pubDate>" .
"<lastBuildDate>Tue, 20 Oct 2008 10:00:00 GMT</lastBuildDate>" .
"<docs>http://www.example.com/rss</docs>" .
"<generator>MyFeed Generator</generator>" .
"<managingEditor>editor@example.com</managingEditor>" .
"<webMaster>webmaster@example.com</webMaster>" .
"<ttl>5</ttl>";
// build the feed...
while ($row = mysql_fetch_assoc($result)) {
$title = $row['title'];
$link = $row['link'];
$description = $row['description'];
$date = $row['date'];
$guid = $row['guid'];
$feed .= "<item>";
$feed .= "<title>" . $title . "</title>";
$feed .= "<link>" . $link . "</link>";
$feed .= "<description> " . $description . "</description>";
$feed .= "<pubDate>" . $date . "</pubDate>";
$feed .= "<guid>" . $guid . "</guid>";
$feed .= "</item>";
}
$feed .= "</rss";
// write the feed out to the server...
echo($feed);
}
?>
|
If you add more to this method, it will soon become virtually unmaintainable.
Good habit: Manageable, focused functions
Listing 4 demonstrates a more concise, more readable rewrite of the original method. In this example, the long method has been broken into smaller methods that each do one thing and do it well. The result will be more reusable in the future and will be easier to test.
Listing 4. Good: Manageable, focused functions
<?php
function createRssHeader()
{
return "<rss version=\"2.0\">" .
"<channel>" .
"<title>My Great Feed</title>" .
"<link>http://www.example.com/feed.xml</link>" .
"<description>The best feed in the world</description>" .
"<language>en-us</language>" .
"<pubDate>Tue, 20 Oct 2008 10:00:00 GMT</pubDate>" .
"<lastBuildDate>Tue, 20 Oct 2008 10:00:00 GMT</lastBuildDate>" .
"<docs>http://www.example.com/rss</docs>" .
"<generator>MyFeed Generator</generator>" .
"<managingEditor>editor@example.com</managingEditor>" .
"<webMaster>webmaster@example.com</webMaster>" .
"<ttl>5</ttl>";
}
function createRssFooter()
{
return "</channel></rss>";
}
function createRssItem($title, $link, $desc, $date, $guid)
{
$item .= "<item>";
$item .= "<title>" . $title . "</title>";
$item .= "<link>" . $link . "</link>";
$item .= "<description> " . $description . "</description>";
$item .= "<pubDate>" . $date . "</pubDate>";
$item .= "<guid>" . $guid . "</guid>";
$item .= "</item>";
return $item;
}
function getUserMaxStories($db_link, $default)
{
$perfsQuery = sprintf("SELECT max_stories FROM user_perfs WHERE user= '%s'",
mysql_real_escape_string($user));
$result = mysql_query($perfsQuery, $db_link);
$max_stories = $default;
if ($row = mysql_fetch_assoc($result)) {
$max_stories = $row['max_stories'];
}
return $max_stories;
}
function writeRssFeed($user)
{
// Get the DB connection information
$settings = parse_ini_file("rss_server.ini");
// look up the user's preferences...
$link = mysql_connect($settings['db_host'], $settings['user'],
$settings['password']) OR die(mysql_error());
$max_stories = getUserMaxStories($link, 25);
// go get my data
$newsQuery = sprintf("SELECT * FROM stories WHERE post_date = '%s'",
mysql_real_escape_string(time()));
$result = mysql_query($newsQuery, $link);
$feed = createRssHeader();
$i = 0;
// build the feed...
while ($row = mysql_fetch_assoc($result)) {
if ($i < $max_stories) {
$title = $row['title'];
$link = $row['link'];
$description = $row['description'];
$date = $row['date'];
$guid = $row['guid'];
$feed .= createRssItem($title, $link, $description, $date, $guid);
$i++;
} else {
break;
}
}
mysql_close($link);
$feed .= createRssFooter();
// write the feed out to the server...
echo($feed);
}
?>
|
Breaking down longer methods can offer diminishing returns, so be careful that when introducing this good habit you don't overdo it. It's possible to break up code so much that it's as difficult to read as it was when it was all one monolithic function.
|
|
Document your code
Documenting your code well sometimes seems as difficult as writing it in the first place. Knowing what to document is tricky because it's tempting to document what the code is doing. Documenting the code's intention is a better idea. In the header blocks of functions that may not be obvious, tell the reader the expected inputs and outputs of the methods and the original intent.
Documenting what the code is doing is common, but unnecessary. If the code is so confusing that you have to document what it's doing, take that as a hint that you should rewrite the code to make it easier to understand. Develop the habit of using good naming and smaller methods and structures to make your code more readable without having to comment what it does.
Bad habit: Missing and redundant function documentation
The comments in Listing 5 merely tell the reader what the code is doing — that it's iterating through a loop or that it's adding a number. But what is missing is why it's doing what it's doing. It would be difficult for someone maintaining this code to know whether the code can be safely changed without introducing new defects.
Listing 5. Bad: Missing and redundant function documentation
<?php
class ResultMessage
{
private $severity;
private $message;
public function __construct($sev, $msg)
{
$this->severity = $sev;
$this->message = $msg;
}
public function getSeverity()
{
return $this->severity;
}
public function setSeverity($severity)
{
$this->severity = $severity;
}
public function getMessage()
{
return $this->message;
}
public function setMessage($msg)
{
$this->message = $msg;
}
}
function cntMsgs($messages)
{
$n = 0;
/* iterate through the messages... */
foreach($messages as $m) {
if ($m->getSeverity() == 'Error') {
$n++; // add one to the result;
}
}
return $n;
}
$messages = array(new ResultMessage("Error", "This is an error!"),
new ResultMessage("Warning", "This is a warning!"),
new ResultMessage("Error", "This is another error!"));
$errs = cntMsgs($messages);
echo("There are " . $errs . " errors in the result.\n");
?>
|
Good habit: Documented functions and classes
The comments in Listing 6 tell the reader the intentions of the classes and methods. The comments tell why the functions are doing what they're doing, which will be much more helpful in the future when the code is being maintained. Conditions may change that require your code to be modified, and that's an easier task if it's simple to find out what your code's purpose was in the first place.
Listing 6. Good: Documented functions and classes
<?php
/**
* The ResultMessage class holds a message that can be returned
* as a result of a process. The message has a severity and
* message.
*
* @author nagood
*
*/
class ResultMessage
{
private $severity;
private $message;
/**
* Constructor for the ResultMessage that allows you to assign
* severity and message.
* @param $sev See {@link getSeverity()}
* @param $msg
* @return unknown_type
*/
public function __construct($sev, $msg)
{
$this->severity = $sev;
$this->message = $msg;
}
/**
* Returns the severity of the message. Should be one
* "Information", "Warning", or "Error".
* @return string Message severity
*/
public function getSeverity()
{
return $this->severity;
}
/**
* Sets the severity of the message
* @param $severity
* @return void
*/
public function setSeverity($severity)
{
$this->severity = $severity;
}
public function getMessage()
{
return $this->message;
}
public function setMessage($msg)
{
$this->message = $msg;
}
}
/*
* Counts the messages with the given severity in the array
* of messages.
*
* @param $messages An array of ResultMessage
* @return int Count of messages with a severity of "Error"
*/
function countErrors($messages)
{
$matchingCount = 0;
foreach($messages as $m) {
if ($m->getSeverity() == "Error") {
$matchingCount++;
}
}
return $matchingCount;
}
$messages = array(new ResultMessage("Error", "This is an error!"),
new ResultMessage("Warning", "This is a warning!"),
new ResultMessage("Error", "This is another error!"));
$errs = countErrors($messages);
echo("There are " . $errs . " errors in the result.\n");
?>
|
|
|
Handle errors
It's been said that when you're writing robust applications, error-handling code seems to follow the 80/20 rule: 80 percent of the code is dedicated to handling exceptions and validations, and 20 percent of the code does the actual work. It's natural when writing code to do happy-path coding. This means writing code that works well for the basic conditions, when all the data is valid and all the conditions are as expected. But such code can be fragile over the application's lifetime. At the other extreme, you may spend too much time writing code for conditions that may never be encountered.
This habit is about finding the balance between doing enough error handling and not doing so much gold plating that your code is never finished.
Bad habit: Not handling errors at all
The code in Listing 7 demonstrates a couple of bad habits. One is not checking the parameters coming in, even though you know at this point that a parameter in a certain state will cause an exception in your method. The second is that the code calls a method that can throw an exception without handling it. This code will leave the author or maintainer to make guesses about the source of the problem — when problems start occurring.
Listing 7. Bad: Not handling error conditions
<?php
// Get the actual name of the
function convertDayOfWeekToName($day)
{
$dayNames = array(
"Sunday",
"Monday",
"Tuesday",
"Wednesday",
"Thursday",
"Friday",
"Saturday");
return $dayNames[$day];
}
echo("The name of the 0 day is: " . convertDayOfWeekToName(0) . "\n");
echo("The name of the 10 day is: " . convertDayOfWeekToName(10) . "\n");
echo("The name of the 'orange' day is: " . convertDayOfWeekToName('orange') . "\n");
?>
|
Good habit: Programming defensively
Listing 8 demonstrates handling and throwing exceptions in a meaningful way. Not only does the additional error handling make the code more robust, but it also helps with readability and understandability. The way the exceptions are handled provides a good indication of what the original author was looking for when the method was written.
Listing 8. Good: Programming defensively
<?php
/**
* This is the exception thrown if the day of the week is invalid.
* @author nagood
*
*/
class InvalidDayOfWeekException extends Exception { }
class InvalidDayFormatException extends Exception { }
/**
* Gets the name of the day given the day in the week. Will
* return an error if the value supplied is out of range.
*
* @param $day
* @return unknown_type
*/
function convertDayOfWeekToName($day)
{
if (! is_numeric($day)) {
throw new InvalidDayFormatException('The value \'' . $day . '\' is an ' .
'invalid format for a day of week.');
}
if (($day > 6) || ($day < 0)) {
throw new InvalidDayOfWeekException('The day number \'' . $day . '\' is an ' .
'invalid day of the week. Expecting 0-6.');
}
$dayNames = array(
"Sunday",
"Monday",
"Tuesday",
"Wednesday",
"Thursday",
"Friday",
"Saturday");
return $dayNames[$day];
}
echo("The name of the 0 day is: " . convertDayOfWeekToName(0) . "\n");
try {
echo("The name of the 10 day is: " . convertDayOfWeekToName(10) . "\n");
} catch (InvalidDayOfWeekException $e) {
echo ("Encountered error while trying to convert value: " . $e->getMessage() . "\n");
}
try {
echo("The name of the 'orange' day is: " . convertDayOfWeekToName('orange') . "\n");
} catch (InvalidDayFormatException $e) {
echo ("Encountered error while trying to convert value: " . $e->getMessage() . "\n");
}
?>
|
Although checking parameters is validation — and it's helpful to anyone using your functions if you require parameters to be a certain way — you should check them and throw meaningful exceptions:
- Handle exceptions as near to the problem as possible.
- Handle each exception specifically.
|
|
Never, ever copy and paste
The ability to copy code from one place and paste it into your code editor is a double-edged sword. On one hand, it saves a lot of errors when you're retyping from an example or a template. On the other hand, it makes proliferation of similar code way too easy.
Be on your guard against copying and pasting code between parts of your application. When you find yourself doing it, stop and ask yourself how you could rewrite the section of code you're copying to be something that you can reuse. Putting code in one place allows you to more easily maintain your code, to a large degree, because changes need to be made in only one place.
Bad habit: Similar sections of code
Listing 9 shows a couple of methods that are nearly identical, except for a couple of values here and there. Tools are available to help you find copied and pasted code (see Resources).
Listing 9. Bad: Similar sections of code
<?php
/**
* Counts the number of messages found in the array of
* ResultMessage with the getSeverity() value of "Error"
*
* @param $messages An array of ResultMessage
* @return unknown_type
*/
function countErrors($messages)
{
$matchingCount = 0;
foreach($messages as $m) {
if ($m->getSeverity() == "Error") {
$matchingCount++;
}
}
return $matchingCount;
}
/**
* Counts the number of messages found in the array of
* ResultMessage with the getSeverity() value of "Warning"
*
* @param $messages An array of ResultMessage
* @return unknown_type
*/
function countWarnings($messages)
{
$matchingCount = 0;
foreach($messages as $m) {
if ($m->getSeverity() == "Warning") {
$matchingCount++;
}
}
return $matchingCount;
}
/**
* Counts the number of messages found in the array of
* ResultMessage with the getSeverity() value of "Information"
*
* @param $messages An array of ResultMessage
* @return unknown_type
*/
function countInformation($messages)
{
$matchingCount = 0;
foreach($messages as $m) {
if ($m->getSeverity() == "Information") {
$matchingCount++;
}
}
return $matchingCount;
}
$messages = array(new ResultMessage("Error", "This is an error!"),
new ResultMessage("Warning", "This is a warning!"),
new ResultMessage("Error", "This is another error!"));
$errs = countErrors($messages);
echo("There are " . $errs . " errors in the result.\n");
?>
|
Good habit: Reusable functions with parameters
Listing 10 shows the code modified to put the copied code into one method. The other methods have been changed to delegate the work to the new method. Building the common method takes some design time, and certainly doing so makes you pause and think instead of instinctively using the copy-and-paste shortcut key combinations. But you'll get back the time you invested the first time the common code needs to be changed.
Listing 10. Good: Reusable functions with parameters
<?php
/*
* Counts the messages with the given severity in the array
* of messages.
*
* @param $messages An array of ResultMessage
* @return int Count of messages matching $withSeverity
*/
function countMessages($messages, $withSeverity)
{
$matchingCount = 0;
foreach($messages as $m) {
if ($m->getSeverity() == $withSeverity) {
$matchingCount++;
}
}
return $matchingCount;
}
/**
* Counts the number of messages found in the array of
* ResultMessage with the getSeverity() value of "Error"
*
* @param $messages An array of ResultMessage
* @return unknown_type
*/
function countErrors($messages)
{
return countMessages($messages, "Errors");
}
/**
* Counts the number of messages found in the array of
* ResultMessage with the getSeverity() value of "Warning"
*
* @param $messages An array of ResultMessage
* @return unknown_type
*/
function countWarnings($messages)
{
return countMessages($messages, "Warning");
}
/**
* Counts the number of messages found in the array of
* ResultMessage with the getSeverity() value of "Warning"
*
* @param $messages An array of ResultMessage
* @return unknown_type
*/
function countInformation($messages)
{
return countMessages($messages, "Information");
}
$messages = array(new ResultMessage("Error", "This is an error!"),
new ResultMessage("Warning", "This is a warning!"),
new ResultMessage("Error", "This is another error!"));
$errs = countErrors($messages);
echo("There are " . $errs . " errors in the result.\n");
?>
|
|
|
Conclusion
If you develop the good habits discussed in this article while developing your PHP code, you'll build code that is easy to read, understand, and maintain. Building easily maintainable code in this manner will enable you to debug, fix, and extend your code with lower risk.
Using good naming and organizing your code into smaller prices makes your code easier to read. Documenting the purpose of your code makes its intent easier to understand and extend. Handling errors properly makes your code more robust. Finally, breaking the crutch of copying and pasting lets you keep your code clean.
discuss this topic to forum
