Bad Code Happens

Every developer knows that sometimes bad code finds its way into even the best products. It doesn't matter how or why; sometimes it just happens. Lack of sleep, lack of knowledge or plain old laziness are often contributing factors (mostly lack of sleep in my case); but every once in a while I stumble across a block of code that goes above and beyond the occasional poorly coded function. Such was the case a few days ago when a client discovered a rather odd error message on one of their websites...

Your theme needs to be fixed for plugins to work. To fix your theme, use the Theme Editor to insert wp_head() just before the line of your theme's header.php file.

So there are several things wrong with this message, some of which should be obvious, some not so much. First of all, you're asking a (presumably) non-technical user to edit their theme! It's true that many plugins don't work correctly without the wp_head() function, but telling the average user they need to go hacking around in their theme or else will likely scare them! There are so many better ways to phrase this message. Perhaps asking them to contact the developer of their theme? Or directing them to your support site for assistance (you do offer support don't you)?

Regardless of the error message itself, there was a larger problem present in this instance, I knew the wp_head() function existed, because I was already using it! So why did this plugin indicate it didn't exist? This mystery warranted some research if only to remove the error and put the customers' mind at ease. A few minutes of digging uncovered the culprit...

function widget_theme_check_wp_head() {
    $template_directory = get_template_directory();
    // If header.php exists in the current theme, scan for "wp_head"
    $file = $template_directory . '/header.php';
    if (is_file($file)) {
        $search_string = "wp_head";
        $file_lines = @file($file);
 
        foreach ($file_lines as $line) {
            $searchCount = substr_count($line, $search_string);
            if ($searchCount > 0) {
                return true;
            }
        }
 
        // wp_head() not found:
        echo "
<div class="highlight" style="width: 99%; margin-top: 10px; margin-bottom: 10px; border: 1px solid darkred;">" . "Your theme needs to be fixed for plugins to work. To fix your theme, use the <a href="theme-editor.php">Theme Editor</a> to insert<code><!--?php wp_head(); ?--></code> just before the <code></code> line of your theme's <code>header.php</code> file." . "</div>
";
    }
} // theme check
add_action('admin_notices', 'widget_theme_check_wp_head');

...wat?

In what world does this even LOOK like a good idea?

First off... there's no rule that says the wp_head() function has to be in header.php. While conventional wisdom does indicate that the wp_head() function logically belongs in the header.php file, many themes place it somewhere else for one reason or another. In this particular instance, the site was using an app theme which had a parent wrapper which contained much of what would normally be considered the "header" code. Could they have written it better (or at least more conventionally)? Sure! But they did include all of the necessities!

Second, why are we iterating through each line of the file performing a literal textual search for wp_head? A textual search seems like a memory-intensive way to accomplish the intended result when the function in question is actually in the anticipated file! There must be a better way to do it...

In fact, there is a better way! Or, more accurately, there are two better ways! One provided by WordPress, and one by PHP itself! So how would we go about handling this check properly? My first thought (and the option used to remedy the situation for this particular client) was through the PHP function function_exists(). The purpose of this function should be self-evident: pass it a function name, and it returns a boolean denoting whether or not the referenced function exists! Using this method, a better way of handling the above monstrosity would be as follows:

function widget_theme_check_wp_head() {
    if ( !function_exists( 'wp_head' ) ) {
        esc_html_e( 'Your theme is missing the wp_head() function! Please contact the theme developer or visit our support forum for help adding it to ensure compatibility.', 'my-text-domain' );
    }
} // theme check
add_action('admin_notices', 'widget_theme_check_wp_head');

Just like that, we've turned a 20 line nightmare into a simple four-line function. It looks cleaner, it works better and, let's face it, is significantly less terrifying to the average user. But I said there was a second option provided by WordPress... what's that about? After discussing this bug with a friend, he pointed out that WordPress offers the did_action() function. This function goes beyond checking the existence of the function and returns the number of times the action has been fired. This function allows us even more control over the output to the extent of only notifying the user (politely) the first time, and not annoying the crap out of them every time they load their dashboard. The same function with this enhancement could be written like this:

function widget_theme_check_wp_head() {
    if ( 1 === did_action( 'wp_head' ) ) {
        echo '<div class="error">' . esc_html__( 'Your theme is missing the wp_head() function! Please contact the theme developer or visit our support forum for help adding it to ensure compatibility.', 'my-text-domain' ) . '</div>';
    }
} // theme check
add_action('admin_notices', 'widget_theme_check_wp_head');

That said, I have to ask myself how a plugin which has 55,000 downloads and a 4.6/5 rating has survived this long without anyone noticing this painfully lousy piece of code. One can't claim inexperience, as they have used function_exists() at least one other place in the same plugin, to say nothing about any of their other work. Lack of sleep? I suppose it's possible, but this isn't an individual, it's a company... QA anyone? I chalk this one up to plain laziness on the part of whoever does their QA and generally poor coding on the part of whoever wrote that function.

So, does bad code happen? Obviously! Nobody's perfect, but that's why quality assurance exists. And good QA isn't limited to companies either. So what if you're an independent developer? QA your work! Run it through its paces before releasing it! Re-read the code and see what can be done better! Almost every piece of software I publish I immediately find room for improvement on a re-read. There's nothing wrong with that; it's called education! Find a mistake now; hopefully you won't make the same mistake next time! Even beyond that simple truth, take it from someone who's been there: it's better if you (or QA) find the bugs and not a user.

/endrant