In the last article, we cleaned up our Snippet code to make it more readable and to meet some standard coding practices. We’ll do some more of that in this article and fix a logic error in the code.
Notice that we’re still not even thinking about adding features to our Snippet. It’s important to have nice, clean, correct code before considering new features. Otherwise, adding the features will be more difficult, and more likely to cause errors. Adding new features to a poorly designed Snippet also tends to force you to redesign it again and again, once for each feature. A well-designed Snippet is much more likely to gracefully accept new features.
Starting Point
For reference, here is the Snippet as it appeared in the last article:
<?php
/* NewResource snippet */
/* Get the publishedon date (if any) and
convert it to a unix timestamp */
$pTime = strtotime($modx->resource->get('publishedon'));
/* Get the current time as a timestamp */
$currentTime = time();
/* Get the resource's age in seconds */
$age = $currentTime - $pTime;
/* Set $interval as the number of seconds in two weeks */
$interval = 1209600;
/* See if the resource is published */
$published = $modx->resource->get('published');
/* Set the return value */
$output = '<span class="new_resource">NEW!</span>';
/* See if publication date is within the last two weeks */
if ($age < $interval ) {
/* Yes, it's recent. If published, return NEW! */
if ($published) {
return $output;
}
} else {
/* Not recent, return an empty string */
return '';
}Logic Trouble
At this point, it’s important to stop and think about whether the Snippet will always do exactly what it’s supposed to. Part of that is thinking about all the situations the Snippet will encounter. In this case the Snippet will be faced with both old and new documents and with published and unpublished documents. That makes four possible conditions: old published, old unpublished, new published, and new unpublished. Take a look at the Snippet above and see if you can spot the problem.
Did you see it? What does the Snippet return if a document is recent but unpublished? If you look closely at the code, you’ll see that the Snippet has no opinion on the matter. It doesn’t return anything in that case. All Snippets should have a return value (unless they never return anything), so it’s a potential problem if one just ends without returning anything at all. We’ll fix that in a bit, but first some thoughts about efficiency.
Code Efficiency
It’s a little early to be thinking about this, and we’re certainly not going to do any optimizations at this point that will make the code less readable, but it’s good to keep this in mind since the logic error is forcing us to do a little redesign.
The principle involved here is simple: Never have your Snippet execute code it doesn’t have to execute. As currently designed, our Snippet is doing pointless calculations to find the age of unpublished Resources. In this case, it’s not really important, since no typical user is every going to have to wait while those calculations happen (since they’ll never see an unpublished Resource). The issue is academic here, but in many Snippets, the difference is critical. For practice, then, while we’re redesigning this Snippet to handle all cases, let’s get rid of that inefficiency.
One Exit Point
While we’re at it, we’ll make another design improvement. It’s not always practical, but a good goal in Snippet design is to give the Snippet a single return statement at the bottom. I will occasionally violate this for the sake of readability, putting a return statement at the very top that skips the whole body of the code if some condition doesn’t apply. In Plugins that handle Resources, for example, I’ll sometimes simply have the Snippet return at the top if the Resource is or isn’t new, though you could make a pretty good case that I shouldn’t do this.
I also sometimes violate the “One Exit Point” principle when I have a very complex, poorly designed section of code that I don’t have time to redesign and test from scratch. In those cases, I’m always sorry that I didn’t spend more time designing the code properly to begin with.
The best way to remind yourself to do this right is to start every new Snippet with this code:
<?php
/* SnippetName snippet */
$output = '';
/* Code goes here */
return $output;Improved Code
/* NewResource snippet */
/* Default return value */
$output = '';
/* Return value for new resources that are published */
$newOutput = '<span class="new_resource">NEW!</span>';
/* Set $interval as the number of seconds in two weeks */
$interval = 1209600;
/* See if the resource is published */
$published = $modx->resource->get('published');
/* For published documents, see if publication date is
within the last two weeks */
if ($published) {
/* Get the current time as a timestamp */
$currentTime = time();
/* Get the publishedon date (if any) and
convert it to a unix timestamp */
$ptime = strtotime($modx->resource->get('publishedon'));
/* Get the resource's age in seconds */
$age = $currentTime - $ptime;
if ($age < $interval) {
/* Yes, it's recent - return NEW! */
$output = $newOutput;
}
}
return $output;This is much better. We’ve got a single exit point at the end, which will always return a value. Since we’ve set it to an empty string at the top of the Snippet, we don’t have to do anything with the $output variable in the Snippet unless we want to change that. This eliminates the else statement. We’ve also wrapped the calculations in if($published) {} so they won’t be performed unless we really need them.
Notice that the variables a user might want to modify are still at the top. The ones we moved are only used for the calculations and would never be set by the user. We had to create a new variable $newOutput, which will be set unnecessarily for unpublished Resources, but putting it here keeps the user from having to search for it in the code below, and setting it will only take a few milliseconds (possibly microseconds), and as we’ll see in the next article, we’re going to want that variable when we start adding features.
The code still has design problem, which we’ll deal with in the next article. It will work perfectly, though, and the design problem could be considered excusable in a quick utility Snippet.
Bob Ray is the author of the MODX: The Official Guide and dozens of MODX Extras including QuickEmail, NewsPublisher, SiteCheck, GoRevo, Personalize, EZfaq, MyComponent and many more. His website is Bob’s Guides. It not only includes a plethora of MODX tutorials but there are some really great bread recipes there, as well.