Clean Code

W3 School 7.4.2014

by Carl Vuorinen / @cvuorinen

What is Clean Code?

What makes code simple and readable?

Naming things

function isMachine($name)
                    

public function isMachine($name)
{
    return str_is($name, gethostname());
}
                    

public function hostnameMatches($name)
                    

public function isCurrentHost($hostname)
                    

What is the meaning of this?

State your intentions.

Say what you mean. Mean what you say.

Minimize guess work and possibility for misinterpretation.

Comments

Why, not What

$all_permutations = array(array());
foreach ($parameters as $parameter => $values) {
    $new_permutations = array();
    // Iterate over all values of the parameter.
    foreach ($values as $value) {
        // Iterate over all existing permutations.
        foreach ($all_permutations as $permutation) {
            // Add the new parameter value to existing permutations.
            $new_permutations[] = $permutation + array($parameter => $value);
        }
    }
    // Replace the old permutations with the new permutations.
    $all_permutations = $new_permutations;
}
                    

$all_permutations = array(array());
foreach ($parameters as $parameter => $values) {
    // To create all possible permutations,
    // combine current values with all existing permutations
    $new_permutations = array();
    foreach ($values as $value) {
        foreach ($all_permutations as $permutation) {
            $new_permutations[] = $permutation + array($parameter => $value);
        }
    }
    $all_permutations = $new_permutations;
}
                    

Avoiding complexity

• Long methods
• Nested control structures
• Classes with many responsibilities
• Too many dependencies
• Too much abstraction
• Overengineering

// Use the user's block visibility setting, if necessary.
if ($block->custom != BLOCK_CUSTOM_FIXED) {
    if ($user->uid && isset($user->data['block'][$block->module][$block->delta])) {
        $enabled = $user->data['block'][$block->module][$block->delta];
    }
    else {
        $enabled = ($block->custom == BLOCK_CUSTOM_ENABLED);
    }
}
else {
    $enabled = TRUE;
}
					

$enabled = TRUE;

if ($block->custom != BLOCK_CUSTOM_FIXED) {
    $enabled = ($block->custom == BLOCK_CUSTOM_ENABLED);

    if ($user->uid && isset($user->data['block'][$block->module][$block->delta])) {
        $enabled = $user->data['block'][$block->module][$block->delta];
    }
}
					

// Limit publicly queried post_types to those that are publicly_queryable
if ( isset( $this->query_vars['post_type']) ) {
    $queryable_post_types = get_post_types( array('publicly_queryable' => true) );
    if ( ! is_array( $this->query_vars['post_type'] ) ) {
        if ( ! in_array( $this->query_vars['post_type'], $queryable_post_types ) )
            unset( $this->query_vars['post_type'] );
    } else {
        $this->query_vars['post_type'] = array_intersect( 
            $this->query_vars['post_type'], 
            $queryable_post_types
    	);
    }
}
					

if (isset($this->query_vars['post_type'])) {
    $this->query_vars['post_type'] = $this->limitPubliclyQueriedPostTypes(
        $this->query_vars['post_type'],
        get_post_types( array('publicly_queryable' => true) )
    );
}
                    

private function limitPublicPostTypes($post_types, array $queryable_post_types)
{
    if (is_array($post_types)) {
        return array_intersect($post_types, $queryable_post_types);
    }

    if (in_array($post_types, $queryable_post_types)) {
        return $post_types;
    }

    return;
}
					

/**
 * Filter array or string to only those values that are allowed
 * 
 * @param array|string $values         String value or an array of values
 * @param array        $allowed_values Values that are allowed
 * 
 * @return array|string Value(s) that are allowed
 */
private function filterArrayOrString($values, array $allowed_values)
{
    if (is_array($values)) {
        return array_intersect($values, $allowed_values);
    }

    if (in_array($values, $allowed_values)) {
        return $values;
    }

    return;
}
					

Object Calisthenics

• One level of indentation per method
• Don't Use The ELSE Keyword
• Wrap all Primitives And Strings
• First class collections
• One dot (arrow) per line
• Don't abbreviate
• Keep all entities small
• No classes with more than two instance variables
• No getters/setters/properties

”Magic”

Unseen control flow makes code harder to follow

• Magic methods
• Events
• AOP

preg_match('/a{5,}rgh/')

preg_match('/^[a-z][a-z\d_]+$/i', $string);
                    

$usernamePattern = "/"
    . "^[a-z]"     // Must begin with a alphabetic character
    . "[a-z\d_]+$" // Then 1-n alphanumeric characters and/or underscores
    . "/i";        // case-insensitive

preg_match($usernamePattern, $username);
                    

} elseif (preg_match('/"([^#"\\\\]*(?:\\\\.[^#"\\\\]*)*)"|\'([^\'\\\\]*
    (?:\\\\.[^\'\\\\]*)*)\'/As', $expression, $match, null, $cursor)
) {
    // strings
    $tokens[] = new Token(
        Token::STRING_TYPE,
        stripcslashes(substr($match[0], 1, -1)), 
        $cursor + 1
    );
    $cursor += strlen($match[0]);
} elseif (preg_match('/not in(?=[\s(])|\!\=\=|not(?=[\s(])|and(?=[\s(])|\=\=\=|\>\=|
    or(?=[\s(])|\<\=|\*\*|\.\.|in(?=[\s(])|&&|\|\||matches|\=\=|\!\=|\*|~|%|\/|\>|\|
    |\!|\^|&|\+|\<|\-/A', $expression, $match, null, $cursor)
) {
    // operators
    $tokens[] = new Token(Token::OPERATOR_TYPE, $match[0], $cursor + 1);
    $cursor += strlen($match[0]);
}
                    

Don't try to be Clever™

Play Golf on your own time

Simplicity is relative

private function getSessionId(Response $response)
{
    /** @var Header $cookies */
    $cookies = $response->getHeader('Set-Cookie');

    return ArrayCollection::create($cookies->parseParams())
        ->map(
            function (array $headerParameters) {
                return isset($headerParameters['SESSION_ID']) ?
                    $headerParameters['SESSION_ID'] :
                    false;
            }
        )
        ->find(
            function ($parameter) {
                return $parameter;
            }
        );
}
                    

private function getSessionId(Response $response)
{
    /** @var Header $cookies */
    $cookies = $response->getHeader('Set-Cookie');

    foreach ($cookies->parseParams() as $headerParameters) {
        if (isset($headerParameters['SESSION_ID'])) {
            return $headerParameters['SESSION_ID'];
        }
    }

    return false;
}
                    

Functional Programming

Immutable variables + functions without state = no side effects

Testing

Tests are a form of documentation

They need to be easily readable

Why?

What's wrong with code that "just works"?

Code is written once, but read many times.

Adding complexity slows you down tomorrow.

Good Enough?

Maintenance vs. Implementation

How?

Ask yourself

Does it make sense?

How well can others understand it?

How could this be easier to understand and use?

Metrics

Static code analysis

Coding Standard

Code Review

Future is the most important thing

Don't make assumptions about the future

Constant Refactoring

Broken window theory

Remove complexity instead of adding it

Hide complexity you cannot remove

Clean code doesn't usually happen on first try.

You have to iterate and refractor.

Further reading

• Clean Code: A Handbook of Agile Software Craftsmanship by Robert C. Martin
• The Art of Readable Code by Dustin Boswell and Trevor Foucher
• Code Simplicity by Max Kanat-Alexander

Further reading

• blog.ircmaxell.com/2013/11/beyond-clean-code.html
• simpleprogrammer.com/2013/04/14/what-makes-code-readable-not-what-you-think/
• blog.ircmaxell.com/2011/03/difference-between-good-and-good-enough.html
• protalk.me/your-code-sucks-lets-fix-it
• blog.ircmaxell.com/2013/05/development-by-numbers-slides.html