Development

The Fundamentals of Writing Clean Code

Writing “clean” code is something we talk about a lot in software development. Developing for the web is no different in the pursuit of code that is “clean” than any other type of programming. It’s also something that you could write a book about (and people have). Without getting into a ridiculous amount of detail, there are some pretty simple best practices to live by that will make your code cleaner right now.

What is “clean” code?

Code that is “clean” is readable, documented, and easy-to-understand. If you are staring at a block of code and you have no freaking clue what it’s doing or how it works, it’s probably not you–it’s the code. “Messy” code can come in many forms, but the result is usually the same: code that is incomprehensible to other developers (possibly even the one who wrote it).

You may be familiar with the phrase “spaghetti code.” Spaghetti code is the kind of code that you come across where, as soon as you start picking at one piece of code, everything else starts falling apart. It’s like opening your kitchen cupboard and having all the contents fall on top of you.

tupperware-fail

Spaghetti code is frequently far too complex for its own good and may have been patched together with solutions to problems that don’t really address the underlying issue. Hopefully, your code doesn’t suffer from these problems but let’s face it, we’ve probably all been there at one time or another.

What’s that smell?

Sometimes you’ll hear developers talk about code “smell”. WTH? What’s a code smell?

Code smell refers to problems in the code or the software program that belie a much deeper issue. If you’ve been using a computer for ten years or more, you’re probably familiar with this:

The dreaded Blue Screen of Death

You’re going about your normal business on your Windows 95 or 98 PC and suddenly your screen turns blue and gives you an indecipherable error message. You are completely baffled. Not only would the error message be unhelpful in telling you what actually happened, it often seemed like you’d see this screen for no reason at all! Randomly your computer would completely freak out and blue screen…usually when you were in the middle of something extremely important and hadn’t saved yet. It’s something even Bill Gates wasn’t immune to during his historic Windows 98 unveiling.

That could be considered an example of code “smell”…you know something’s wrong, but you might not know exactly what yet, and digging into it only leads to a series of rabbit holes that suck you deeper and deeper. Code smell is when you’re looking at a piece of code and think “Hey something’s not right here.” Spaghetti code has a “smell,” but I’ve seen code that was otherwise written cleanly, and still had a smell because it was trying to do too many things.

Clean code, ideally, won’t have a “smell.” These are a few tips I’ve learned for keeping your code from being smelly.

White space is your friend

One way you can make your code immediately more readable is adding more space between blocks of code. Now, I’m not saying  you need to hit return after every

single

variable,

but it’s a known fact in the world of design (I know, not a place we code monkeys spend a lot of time) that more white space makes things feel more open and readable. Compare these two code blocks:

function wds_current_user_can_view_doc($post_id,$user_id) {
    if (!$post_id){return false;}
    $post = wds_ce_get_doc($post_id);
    if ('publish' == $post->post_status){return true;}
    if (current_user_can('manage_options')){return true;}
    if (!$user_id){$user_id = get_current_user_id();}
    if ($post->post_author == $user_id){return true;}
    if (current_user_can('edit_others_dms_documents')){return true;}
    if (current_user_can('view_others_dms_documents')){return true;}
    return false;
}
function wds_current_user_can_view_doc( $post_id, $user_id ) {

    if ( ! $post_id ) {
        return false;
    }

    $post = wds_ce_get_doc( $post_id );

    if ( 'publish' == $post->post_status ) {
        return true;
    }

    if ( current_user_can( 'manage_options' ) ) {
        return true;
    }

    if ( ! $user_id ) {
        $user_id = get_current_user_id();
    }

    if ( $post->post_author == $user_id ) {
        return true;
    }

    if ( current_user_can( 'edit_others_dms_documents' ) ) {
        return true;
    }

    if ( current_user_can( 'view_others_dms_documents' ) ) {
        return true;
    }

    return false;
}

What changed? In the first example, we have no line breaks between the different conditionals, and the return values are on the same line as the condition. It saves space, sure, but it also feels cramped and it becomes difficult to read. It feels like there’s a lot of stuff going on and I don’t have the patience to go through every line and figure out what it all does.

All we did in the second example was add some space. Give your functions and conditionals some room to breathe. Here’s another example:

function do_messages( $message_id = 0, $message_string = '' ) {

    if ( ! $message_id && '' == $message_string ) {
        return;
    }

    switch ( $message_id ) {
        case 0 :
            echo '<div class="error clear"><p>' . $message_string . '</p></div>';
            break;

        case 1 :
            echo '<div class="error clear"><p>' . __( 'Document not deleted. The document could not be found.', 'wds-ce-dms' ) . '</p></div>';
            break;

        case 2 :
            echo '<div class="error clear"><p>' . __( 'Could not perform the requested action.', 'wds-ce-dms' ) . '</p></div>';
            break;

        case 3 :
            echo '<div class="error clear"><p>' . __( 'You do not have permission to delete this document.', 'wds-ce-dms' ) . '</p></div>';
            break;

        case 4 :
            echo '<div class="updated"><p>' . __( 'Document deleted successfully. The document was deleted permanently and cannot be restored.', 'wds-ce-dms' ) . '</p></div>';
            break;

        case 5 :
            echo '<div class="updated"><p>' . __( 'Document trashed successfully. The document may be deleted permanently by an admin.', 'wds-ce-dms' ) . '</p></div>';
            break;

        default:
            break;
    }

}

This is a simple function that spits out a message based on a value that’s passed into it. You’ll notice that for each case, everything that is conditional to that particular case is grouped together. We can immediately identify the message that displays if a message ID of 1 is passed to the function, the message that displays if the ID of 2 is passed. It flows and it makes sense and it doesn’t need a lot of documentation to explain what it’s doing. We’re not putting line breaks between every line of code, but there are a good amount of breaks between snippets within a function.

WordPress coding standards say that when passing parameters to a function, you should always put a space between the parenthesis or brackets and the value you are passing. Take the example of the current_user_can function. I could write a current_user_can check like this:

current_user_can('manage_options');

That’s perfectly correct, programmatically. Even from a code smell point of view, there’s nothing wrong with that line of code. However, this becomes more readable:

current_user_can( 'manage_options' );

White space is like oxygen. Without breaks or empty space, your code will suffocate. Ever tried to read minified JavaScript or CSS? Good luck. It hurts my head just to look at it. On the other hand, looking at the raw, unminified code is like a breath of fresh air.

Think of each block of code as a paragraph of text. Each line of code is a sentence. Within each sentence, there are phrases — these are your individual functions. There’s a reason why the WordPress mantra is Code is poetry. Just like writing a blog post, your code shouldn’t be full of run on sentences, no punctuation and wordsrunningtogether. Instead, your code can be as beautiful, as elegant and as simple as a haiku.

Keep it simple, stupid

KISS

The second fundamental rule of clean code is writing small functions that do one thing. Or, at least, as few things as possible. You have a function and there’s half a dozen nested if/else conditions? You’re __doing_it_wrong(). One rule of thumb (pulled out of the Clean Code book I linked to at the beginning of this post) is if you can’t explain what your function does in one sentence or less, it’s doing too many things.

It could be argued that the do_messages function posted above is doing too many things because it’s using a PHP switch. By default, simply because we’re using a switch, we know it’s going to be doing a bunch of different things. Sometimes you can’t get around doing a couple different things inside your function. I would argue that, since that do_messages function is immediately returning a message string and nothing else, it’s still pretty clean. My one sentence to describe the function would be “takes a message ID and returns a corresponding string of text.”

Look, I know it’s hard to write small functions. Especially if you’ve gotten into the habit of writing these sprawling functions that are super complex and do lots of things. And maybe you know exactly how they work and where x plugs into y. But as WordPress developers, we are a community, and we are quite often not the only ones looking at our code. You might understand what that 500 line function does, but I don’t, and when a single function is that big, it makes it harder to read. And the thing is, if you look at that 500 line function, or a 1000 line function, or a (God forbid) 2000 line function, you will start to see places where you could break it up. Take this chunk here and put it into its own function. Not only does it make your code cleaner, but it makes it easier to reuse blocks of code later.

Breaking large functions into smaller, simpler ones also makes it easier for other coders to come in behind you and easily grok what your code is doing. As one of our dev leads, Justin Sternberg said on our developer call just this last week “always think about the dev coming in behind you.” That person might be coming from a completely different place, with different experience, and they might know nothing about your code or what this function is doing. Do them a favor and split your code into smaller, easier-to-digest blocks.

Smaller functions are also better for debugging. Let’s say you did something stupid like what I did last week: you forget to add a ! to check a particular variable. That is going to be a huge pain to try to track down in a 2000 line function, but if your function is only 5 lines, it should be a snap.

Clean code is as minimalist and functional as your IKEA office furniture. You know what sets IKEA furniture apart from other furniture? Fewer parts, simpler construction. That’s what your code should strive to be.

Document your code and write self-documenting code

The first time I heard both these things come out of the same person’s mouth, I thought they were nuts. It seems totally contradictory, right? Either your code is documented, or it’s self-documenting. How can you have both things at the same time? Here’s the actual code from those first two code examples in a project we are working on:

/**
 * Function to check if the passed user should be able to view a document
 *
 * @param  int  $post_id     The document we're editing
 * @param  int  $user_id     The user we're checking
 * @return bool
 */
function wds_current_user_can_view_doc( $post_id = 0, $user_id = 0 ) {

    // post id is REQUIRED
    if ( ! $post_id ) {
        return false;
    }

    // get the post object for the passed post id
    $post = wds_ce_get_doc( $post_id );

    // if the post is published, return true
    if ( 'publish' == $post->post_status ) {
        return true;
    }

    // if the current user is an admin, return true
    if ( current_user_can( 'manage_options' ) ) {
        return true;
    }

    // if no user id is passed, assume the current user
    if ( ! $user_id ) {
        $user_id = get_current_user_id();
    }

    // if the author of the post is the passed (or current) user, they can view
    if ( $post->post_author == $user_id ) {
        return true;
    }

    // if the current user can edit other users' documents, they can view
    if ( current_user_can( 'edit_others_dms_documents' ) ) {
        return true;
    }

    // if the current user can view other users' documents, they can view
    // NOTE: this capability doesn't exist (yet)
    if ( current_user_can( 'view_others_dms_documents' ) ) {
        return true;
    }

    // all others return false
    return false;
}

It’s easy to see what’s changed, right? There’s inline documentation. There’s a docblock at the top, which tells you what the function does, what the parameters are and what they’re for, and what type of value the function returns. There’s also comments for each check that say what each thing is for. You should be able to walk through the function and figure out pretty quickly what we’re doing here. None of those comments are absolutely essential. You could probably figure them out without the comments. But the comments help you along. There’s even a comment explaining a particular check against a capability that isn’t in use yet — so if and when that feature is added in the future, you (or the next developer) will know how it’s expected to work. Especially with something like this, where there’s a bunch of different conditions, inline documentation help not only future devs looking at your code, but you looking at your code two weeks from now, to understand what your code is doing.

That’s documenting your code, and that’s easy to grasp. “Self-documenting code” might be a bit more challenging to understand. And maybe how I define self-documenting code is totally different than how you think about self-documenting code. That’s okay, too.

Here’ s a super-simple example of self-documenting code:

function __return_true() {

     return true;

}

There’s no question about what this function does. It’s called __return_true and it returns true. You’d be surprised if you used the __return_true function and got a false, right? That totally wouldn’t make any sense. Self-documenting code tells you what it does so that there should be no question. In my previous code example, my function is named wds_current_user_can_view_doc. What does it do? Well, probably it figures out whether a user can view a document. It’s implied from the name. I could have named the function user_doc_check, but if I did, would it be as easy to understand as current_user_can_view_doc? Probably not.

There was a time when computers weren’t nearly as powerful as they are now, and it was frowned upon to use really long function and variable names in code because memory was limited, and each additional character used up a valuable byte. But even now, sometimes I’ll see stuff like this:

foreach ( $posts as $p ) {
     echo sprintf( 
          '<li><a href="%1$s">%2$s</a> (%3$s)</li>',
          $p->guid,
          $p->post_title,
          $p->comment_count
     );
}

What’s the point of using $p to describe your post variable than $post? Okay, so maybe if you used $post, specifically within a WordPress environment, it might clash with the $post global, but that doesn’t stop you from using something unique, like, say $single_post. Let’s look at that again using $single_post:

foreach ( $posts as $single_post ) {
     echo sprintf( 
          '<li><a href="%1$s">%2$s</a> (%3$s)</li>',
          $single_post->guid,
          $single_post->post_title,
          $single_post->comment_count
     );
}

Now I can read that block of code like a sentence. I have a group of posts, and for each post, I’m going to echo something about each single post. Variables can be as long as you want to make them. There’s no reason to shorten them.

But, Chris, using $p is faster to type!

Okay, sure, short functions and variable names might save you a couple seconds typing, but most decent IDEs will recognize when you’ve used a function or variable name in your code before and autofill it for you, so you’re not really losing that much time. And, if you seriously need to shorten your variables to one letter to optimize your coding as much as possible, you’re going way too fast anyway. Just like it takes time to think about and write a good blog post (ahem), writing good code should take time, too. It shouldn’t be let’s crank this out as quickly as possible because that approach leads to more mistakes.

Think about the best documentation you’ve ever seen. What does it look like? Is it the manual that came with your circa-1990s VCR that told you how to program a record time to watch your favorite show? Probably not. For me, the best documentation looks like this:

Lego Instructions

The best part of this documentation is that there are no words at all. It’s self-documenting. But even so, there’s a clear destination (you know what you’re expected to do), and the steps are broken out into small pieces that are easy to understand. And yet, despite how simplistic the documentation is, you can build complex things like this:

Deather

Instead of writing $ds and assuming you remember what that stands for in six months, be explicit in your naming conventions–write out $death_star so it’s obvious what you’re referring to.

Get func-y

James-Brown_1973

In line with keeping things simple, when writing functions, try to limit the number of parameters you pass to your function as much as possible, and always set defaults. This:

function wds_my_cool_function( $things = array(), $count = 0 )

instead of this:

function wds_my_uncool_function( $thing_1, $thing_2, $thing_3, $thing_4, $count, $types, $fruit, $animals, $kitchen_sink )

Limiting the number of parameters your functions accept will help you keep your functions simple. The rule of thumb is three is the absolute most, and even then it should be an exception. More than that and your function is probably doing too many things. If you needed to pass as many parameters as there are in wds_my_uncool_function, you could easily strip that down to one, like this:

function wds_my_cooler_function( $args = array() )

BOOM. Now, I can pass as many variables as I want into that one $args parameter, so there’s no reason to have a function that takes nine individual parameters.

Giving your function parameters defaults serves two purposes: it allows you to fail or break out if you have missing or weird input and it gives your parameters context. When I look at wds_current_user_can_view_doc( $post_id = 0, $user_id = 0 ), I know that both the $post_id and the $user_id should be integers. I don’t need the docblock to tell me that. I know, just from the function declaration, how to deal with that type of data inside the function and I know how to check for the type of data that I should be getting. If I got a string instead of an integer in one of those variables, I know something is wrong, and this can prevent type problems in your code later.

The default values you give your variables should reflect the data type those variables will be given. This isn’t a requirement in PHP–you can assign a variable a value of one type, and then change it later and it will be perfectly fine.

$thing = 1;

/* ... some code ... */

$thing = 'banana';

But this is confusing for developers and can lead to bugs when your code is expecting one type of data and you’re giving it another. However, if you have a function parameter and no value was passed to it, it will result in an error. Setting it to at least some kind of empty value will prevent errors.

The other thing you can do, since now you’re giving values to your function parameters, is set those parameters to whatever you want the default value to be. The following example, pulled from my Book Review Library plugin, is loosely based on the get_the_term_list function in WordPress core:

/**
 * Get genres
 * returns a formatted list of genres (comma-separated by default) with links to each
 *
 * @since  1.0.0
 *
 * @param  $before     string     string to display before the genre
 * @param  $after      string     string to display after the genre (comma by default)
 * @param  $forced     boolean    by default, if the item is the last in the list, the $after variable
 *                                doesn't render. If $forced is set to TRUE it will bypass this and render 
 *                                it anyway (e.g. if passing $before = '<li>' / $after = '</li>')
 * @return $genre_list            sanitized string of the results
 */
function get_genres( $before = null, $after = ', ', $forced = false ) {

    global $post;

    $genres = get_the_terms( $post->ID, 'genre' );
    $genre_list = null;

    if ( $genres && !is_wp_error( $genres ) ) {

        $genre_out = array();

        foreach ( $genres as $genre ) {
            $genre_out[] = sprintf( '<a href="%s">%s</a>',
                home_url() . '/?genre=' . $genre->slug,
                $genre->name );
        }

        $count = 0;

        foreach ( $genre_out as $out ) {
            $genre_list .= $before . $out;
            $count++;
            if ( ( count( $genre_out ) > 1 ) && ( $after == ', ' ) && ( count( $genre_out ) != $count ) || $forced ) {
                $genre_list .= $after;
            }
        }

    }

    if ( $genre_list ) {
        return wp_kses_post( $genre_list );
    }

}

Instead of just passing empty values to the function and setting the defaults inside the function, I’m setting the defaults at the same time I’m adding the parameters to the function and maybe changing them later.

Formatting arrays and repetitive variable declarations

This one is purely cosmetic. Before joining WebDevStudios, if I was going to, for example, register a new post type, I might write something like this:

$capabilities = array(
    'publish_posts' => 'publish_book-reviews',
    'edit_posts' => 'edit_book-reviews',
    'edit_others_posts' => 'edit_others_book-reviews',
    'delete_posts' => 'delete_book-reviews',
    'edit_post' => 'edit_book-review',
    'delete_post' => 'delete_book-review',
    'read_post' => 'read_book-review'
);
$labels = array(
    'name' => __( 'Book Reviews', 'book-review-library' ),
    'singular_name' => __( 'Book Review', 'book-review-library' ),
    'add_new' => __( 'Add New', 'book-review-library' ),
    'add_new_item' => __( 'Add New Book Review', 'book-review-library' ),
    'edit_item' => __( 'Edit Review', 'book-review-library' ),
    'new_item' => __( 'New Book Review', 'book-review-library' ),
    'view_item' => __( 'View Book Review', 'book-review-library' ),
    'search_items' => __( 'Search Book Reviews', 'book-review-library' ),
    'not_found' => __( 'No book reviews found', 'book-review-library' ),
    'not_found_in_trash' => __( 'No book reviews found in Trash', 'book-review-library' ),
    'menu_name' => __( 'Book Reviews', 'book-review-library' ),
);
$args = array(
    'labels' => $labels,
    'hierarchical' => false,
    'description' => 'Book Review',
    'supports' => $supports,
    'taxonomies' => array( 'genre', 'review-author' ),
    'public' => true,
    'show_ui' => true,
    'show_in_menu' => true,
    'menu_position' => 20,
    'show_in_nav_menus' => true,
    'publicly_queryable' => true,
    'exclude_from_search' => false,
    'has_archive' => true,
    'query_var' => true,
    'can_export' => true,
    'rewrite' => true,
    'capability_type' => 'book-review',
    'capabilities' => $capabilities,
    'map_meta_cap' => true
);
register_post_type( 'book-review', $args );

Now there’s nothing wrong with this at all. But this is a lot easier to read:

$capabilities = array(
    'publish_posts'       => 'publish_book-reviews',
    'edit_posts'          => 'edit_book-reviews',
    'edit_others_posts'   => 'edit_others_book-reviews',
    'delete_posts'        => 'delete_book-reviews',
    'edit_post'           => 'edit_book-review',
    'delete_post'         => 'delete_book-review',
    'read_post'           => 'read_book-review'
);
$labels = array(
    'name'                => __( 'Book Reviews', 'book-review-library' ),
    'singular_name'       => __( 'Book Review', 'book-review-library' ),
    'add_new'             => __( 'Add New', 'book-review-library' ),
    'add_new_item'        => __( 'Add New Book Review', 'book-review-library' ),
    'edit_item'           => __( 'Edit Review', 'book-review-library' ),
    'new_item'            => __( 'New Book Review', 'book-review-library' ),
    'view_item'           => __( 'View Book Review', 'book-review-library' ),
    'search_items'        => __( 'Search Book Reviews', 'book-review-library' ),
    'not_found'           => __( 'No book reviews found', 'book-review-library' ),
    'not_found_in_trash'  => __( 'No book reviews found in Trash', 'book-review-library' ),
    'menu_name'           => __( 'Book Reviews', 'book-review-library' ),
);
$args = array(
    'labels'              => $labels,
    'hierarchical'        => false,
    'description'         => 'Book Review',
    'supports'            => $supports,
    'taxonomies'          => array( 'genre', 'review-author' ),
    'public'              => true,
    'show_ui'             => true,
    'show_in_menu'        => true,
    'menu_position'       => 20,
    'show_in_nav_menus'   => true,
    'publicly_queryable'  => true,
    'exclude_from_search' => false,
    'has_archive'         => true,
    'query_var'           => true,
    'can_export'          => true,
    'rewrite'             => true,
    'capability_type'     => 'book-review',
    'capabilities'        => $capabilities,
    'map_meta_cap'        => true
);
register_post_type( 'book-review', $args );

Similarly, if I’m assigning values to a bunch of variables, I might’ve written it like this:

$singular = $args['singular']; // required
$plural = $args['plural'];   // required
$slug = $args['slug'];     // required
$show_ui = ( isset( $args['show_ui'] ) ) ? $args['show_ui'] : true;
$show_in_nav_menus = ( isset( $args['show_in_nav_menus'] ) ) ? $args['show_in_nav_menus'] : true;
$tagcloud = ( isset( $args['show_tagcloud'] ) ) ? $args['show_tagcloud'] : true;
$hierarchical = ( isset ( $args['hierarchical'] ) ) ? $args['hierarchical'] : true;
$name = ( isset( $args['use_singular_labels'] ) && $args['use_singular_labels'] ) ? $singular : $plural;

…but this looks a lot nicer:

$singular          = $args['singular']; // required
$plural            = $args['plural'];   // required
$slug              = $args['slug'];     // required
$show_ui           = ( isset( $args['show_ui'] ) ) ? $args['show_ui'] : true;
$show_in_nav_menus = ( isset( $args['show_in_nav_menus'] ) ) ? $args['show_in_nav_menus'] : true;
$tagcloud          = ( isset( $args['show_tagcloud'] ) ) ? $args['show_tagcloud'] : true;
$hierarchical      = ( isset ( $args['hierarchical'] ) ) ? $args['hierarchical'] : true;
$name              = ( isset( $args['use_singular_labels'] ) && $args['use_singular_labels'] ) ? $singular : $plural;

It goes back to the whole white space is oxygen thing. It’s much easier to read (and it just looks nicer) to write your array values or repetitive variable assignments so they are aligned the same. I’d argue that you can actually see what’s going on more with the consistent spacing. What I will usually do is write the longest variable or array key name first, so I know where my => or = is going to be, and then align all my other values around that one normally with spaces.

Spaces are better than tabs for this because different IDEs will handle tabs and spaces differently; using spaces ensures that it will look the same no matter what IDE is being used. Does it take more time to do this? Sure it does. But, again, coding isn’t a race. Writing good code takes time and giving yourself time to write readable code might reveal something that could have gone unnoticed if you rush it.

Return early and often

Finally, a good rule of thumb for writing clean code is to “return early and often”. This means that if you’re doing some check, don’t make your reader have to process a whole bunch of code to get to a failure notice or a return. Take the following example:

if ( $condition_was_met ) {

     do_something();

} else {
     
     // condition was not met
     return;

}

Let’s say I have a lot more going on than just do_something(). That could be 20 lines of code that only executes if the condition was met. If we’re thinking linearly, it’s natural to want to deal with what happens when the condition is met first. To take Newton’s observation about gravity as an example, it would be counter-intuitive to think about what doesn’t happen when an apple falls from a tree.

However, if the condition isn’t met, I need to read through those 20 lines of code about something that does not apply to me if I’m only concerned with the else condition. Consider rewriting that function like this:

if ( ! $condition_was_met ) {

     // return early if check failed
     return;

}

// now that we know that the condition was met, 
// everything that follows is stuff that happens for that condition
do_something();

If I’m looking for what happens if the condition isn’t met, I get that right away. But most of the time, I’m more concerned with what happens when the condition is met. Once the failure is out of the way, I know that everything following the ! $condition_was_met clause is stuff that’s going to happen as long as the condition is met. Additionally, since we’ve gotten what happens when the condition isn’t met out of the way, I don’t have to constantly think back to what my condition was.

Any time you’re inside a curly bracket, you have to think about what that curly bracket represents.
— Justin Sternberg

Returning early means that if the condition isn’t met, we just stop, we’re done, do not pass Go, do not collect $200, and don’t worry about the rest of the code in this function.

Get to coding!

This is by no means an exhaustive list of ways to clean up your code. But all of these are relatively simple tweaks in your process that can go a long way to writing clean code. I owe a lot to Greg Rickaby, whose onboarding in my first week at WebDevStudios forced me to think more about how my code was written, and who recommended–nay, demanded–that I get a copy of Clean Code because it would “change your life.”

In addition to that, I recommend checking out the Code Standards section of the WordPress Core Developer Handbook for further reading. In addition, 10up and the WordPress.com VIP team have written excellent best practice guides for development.

13 thoughts on “The Fundamentals of Writing Clean Code

  1. Very nice article, I faced blue death screen more during my life.
    Following rules of writing clean code is a habit, I always try to follow these rules as possible as I can.

  2. I think you misunderstand the idea of a code smell. A BSOD is not a code smell, it is a bug. The exact causes vary, but its pretty hard to determine without a debugger and core dump.

    A code smell is something you can see when you look at code that, through experience, you know is often a source of bugs. Like smelling something funky in the real world, it may not be a cause of concern, but is probably worth investigating and seeing why it is that way. If it turns out to be benign, add a comment so the next person to read the code doesn’t waste time the same way.

    1. Interested, you don’t think that smelly code was a contributor to blue screens? I imagine if the code was cleaner, so that more developers who looked at it could read and understand what was going on, then there would have been less problems with BSODs. Obviously, I can’t get into those guys’ heads, and I’ve never looked at the Windows 9x source code, so it’s only speculation, but that’s the only point I was trying to make.

  3. I’d also heavily recommend against:

    1. Providing defaults for most parameters.
    2. Passing multiple arguments in an array.

    Any decent code analysis tool will produce warnings of these two. It may be that PHP makes defaults OK in some circumstances, (e.g. no overloading), but stuffing args into an array loses all readability. Better refactor differently.

    I’d highly recommend reading Martin Fowler’s Refactoring. It would warn you against these and suggest better workarounds.

    1. I agree that passing an array of arguments is generally a bad idea. However, it’s far better to do that — and deal with the array arguments within the function — than passing a million parameters to the function individually, as in the example. There are many instances in WordPress core where multiple arguments for things like WP_Queries are passed in an array of arguments that gets parsed by the WP_Query class. As this was written from a WordPress perspective, I tend to default to the WordPress Coding Standards and how core handles things.

      That said, there are probably very few instances when passing an array to a function as a parameter would be a good idea, definitely better to find another way of doing it. But if your functions are simple, and only doing one thing, probably you wouldn’t wind up in that situation, anyway. (Because how can you only be doing one thing when you’re passing 9 different values to your function?)

      Using a function that takes parameters that don’t have defaults defined will throw an error if the parameter wasn’t passed no matter what. If I’m a new developer, trying to use this function from some plugin that I didn’t write, I might not know about the required parameters and then have to spend time looking at source code to try to figure out what I did wrong (and hope that there’s documentation in the code explaining that). IMO, it’s better to define defaults for your function parameters (if your functions need parameters at all) and then deal with those cases where nothing was passed within the function (e.g. defining a more descriptive error like “you forgot to pass a value to $x”) or using a default value so the function can still be used even if nothing was passed.

  4. Great article Chris!

    Some of my first programs were on those old systems where you had to limit your characters. I am definitely guilty of shortening $single_post to $p.

    1. Thanks, Ben!

      I remember when I was first starting out, finding these loops with shortened variables and having no idea what any of them were. It can be super confusing (and unnecessary) especially when you’re first starting out.

  5. Thanks for the list Chris, I’ve been using return early an ofter in my code for a while and love this simplicity.

    P.S. I just got a copy of clean code 🙂

  6. Well done! Having been guilty of a number of bad practices it’s nice to know others have been there too. The examples and explanations are on point and I still picked up some good nuggets here.

  7. Love this post. Finding the right balance between fast code and well documented code can be an art form at times, especially with the nuances between all the languages out there. And working with a platform like WordPress where you frequently have to dive into the underlying PHP, I always appreciate authors who document and organize their code. A little extra work up front can save a lot of time for someone else later on.

Have a comment?

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

accessibilityadminaggregationanchorbackupsbookmarksbuddypresscachingcalendarcaret-downcartunifiedcrediblecustommigrationdesigndevecomfriendsgoodgroupsgrowthhostingideasinternationalizationiphoneloyaltymailhealthmessagingArtboard 1migrationsmultiple-sourcesmultisitenotificationsperformancephoneprofilesresearcharrowscalablescrapingsecuresharearrowarrowsourcestreamsupportunifiedupdatesvaultwebsitewordpress