DE LabsDE LabsDE LabsDE Labs
  • About
  • Dominion Enterprises
  • DE Hackathon
  • Careers
  • Submissions

Clever Code is Bad Code

November 25, 2015Chris RyanCoding Pro Tips1 comment

Clever is defined as someone who is intelligent and able to learn things quickly, marked by wit or ingenuity. For a developer these are desirable qualities. Clever code happens when we are trying to solve a problem in a manner that uses some obscure functionality, compacts or reduces the size of code, or hides its intent in any way. For our code sometimes clever is good, many times it is not.

Oh I get it, it's very clever.

Why does clever code happen? Often it is because, as developers, we want to demonstrate how smart we are. The best way to do this is with code that performs multiple actions in a single step or in some novel way. This also leads to code that is hard to understand for others and even ourselves. After some time has passed what we thought was clever turns out to be quite the opposite.

Here is an example of some PHP code that I found that someone thought was very clever when they wrote it.

array_key_exists($word, $wordsFound) ? $wordsFound[$word] += count($matches) : $wordsFound[$word] = count($matches);

The largest problem I see is how difficult it is to understand what the code is doing at a glance. Be honest; how long did you have to look at that code before you started to understand what it was doing? The code can be refactored to accomplish the same goals while being much easier to read and understand.


“remove the cleverness”

if (array_key_exists($word, $wordsFound)) {
    $wordsFound[$word] += count($matches);
} else {
    $wordsFound[$word] = count($matches);
}

Here is an example using “continue” in nested loops.

foreach ($routes as $route => $controllerAction) {
    $routeComponents = array_filter(explode('/', $route));
    // ...
    foreach ($routeComponents as $i => $routeComponent) {
        if ($routeComponent == '(.*)') {
            continue;
        } elseif ($routeComponent != $pathComponents[$i]) {
            continue 2;
        }
    }
    return $controllerAction;
}

The use of “continue” and “continue 2” are obscuring the actual intent and make it harder to determine the flow of the code. This solution was likely chosen as it allowed for the desired outcome without adding a lot of code or rewriting the function. After some inspection I determined the sub-loop is comparing an expected route with a path, taking into consideration a wild card match. If none of the continues are called it eventually falls through and returns the chosen value.

function routeAndPathMatch($routeComponents, $pathComponents) {
    foreach ($routeComponents as $i => $routeComponent) {
        if ( $routeComponent != '(.*)' &&
            $routeComponent != $pathComponents[$i]) {
            return false;
        }
    }
    return true
}

foreach ($routes as $route => $controllerAction) {
    $routeComponents = array_filter(explode('/', $route));
    // ...
    if (routeAndPathMatch($routeComponents, $pathComponents)) {
        return $controllerAction;
    }
}

Using a separate function for the sub-loop we can remove the cleverness and make the code much easier to understand. Another advantage of this code is the ability to test the new function independently from the rest of the code.

How can we prevent clever code? When writing code think about the next person who will look at it, or you a year later. Will you have to explain it to someone or look up documentation to verify a specific behavior? Be proactive and ask someone to review your code and see how long it takes them to figure out what it is doing. Simple code should be easily understood as you read it! Also consider alternative ways to write the same code that may express the intended behavior in a more straightforward way that improves readability.

Related

Chris Ryan
Chris is the Lead Developer for the Dominion Web Solutions Recreational Brands and has been with Dominion for more than 4 years. With more than 15 years of software development experience, Chris has worked on numerous projects ranging from backend devops, website development and maintenance, to front end desktop software applications. Chris was first introduced to programming in Basic on a Commodore 64 and later learned C/C++ programming. Today Chris primarily develops using the PHP and Javascript languages.
Previous post The 12th Labor of a .NET Developer Part 1 Next post The 12th Labor of a .NET Developer Part 2

1 comment. Leave new

Sam Littler
December 14, 2015 7:42 am

I absolutely agree that you shouldn’t try to be too clever when coding, but I think your first example is a poor one. To me, it’s fairly transparent what’s happening there.

Reply

Leave a Reply Cancel reply

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

  • About
  • Dominion Enterprises
  • DE Hackathon
  • Careers
  • Submissions
© 2016 Dominion Enterprises