Testing Spaghetti

Gemma Lynn / @ellotheth

Hi, I'm Gemma

You don't know me, but sometimes I work on fun things:

  • Cornell University's NASA research center
  • Postal mailing industry (ok, so fun is a minor exaggeration here)
  • WonderMinion-hood

I'm mostly boring.

Let's talk about pasta!

You are here

Large monolithic programs are like a plate of spaghetti...

...pull it here and something moves on the other side.

— Dennie Van Tassel, 1974

spaghetti code, n.

Code having the same clean logical structure as a plate of spaghetti.

— Richard Conway, 1978

Spaghetti code is incomprehensible unless one has the discipline to follow each noodle through from one side to the other.

— Michael O. Church, 2012 


label_b:

if ($condition_a) {
    goto label_a;
}

while ($condition_b) {
    if ($condition_c) {
        goto label_b;
    }
}

label_a:

echo 'Well, that was exciting.';

Macaroni code

Macaroni

Languages all mixed together like mac & cheese

SQL in all the things!


<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Transitional//EN"
     "http://www.w3.org/TR/xhtml1/DTD/xhtml1-transitional.dtd">
<html>

<?
$res = mysql_query("select title from pages where name = 'index'");
if ($res) {
    $title = mysql_fetch_assoc($res)[0]['title'];
} else {
    $title = 'Default title';
}
?>

<head>
    <?= $title ?>
</head>
                        

<body>
<?
$res = mysql_query("select * from posts order by date desc limit 10");
if (!$res) {
    echo "
Whoops, no posts!
";
} else {
    while ($post = mysql_fetch_assoc($res)) {
?>
        

            
<?= $post['title'] ?>

            
<?= $post['body'] ?>

        

<?
    }
}
?>
</body>

common.php


function getUserById($id) {
    if ($id >= 0) {
        $res = mysql_query("
            select * from users
            where id = $id
        ");
        if ($res) {
            return mysql_fetch_assoc($res)[0];
        }
    }
    return null;
}
                        

function getUserByNameAndPassword($name, $password) {
    if ($name) {
        $res = mysql_query("
            select * from users
            where id = $id and password = '$password'
        ");
        if ($res) {
            return mysql_fetch_assoc($res)[0];
        }
    }
    return null;
}
                        

function saveNewUser($user) {
    if (strlen($user['password']) > 5) {
        return false;
    }
    if (!$user['name'] && !$user['password']) {
        return false;
    }
    // ... more business logic ...

    mysql_query("
        insert into users (name, password)
        values ({$user['name']}, {$user['password']}
    ");
}

Frameworks do not guarantee spaghetti-free code


$bullet_app->path('/pets', function() use ($bullet_app) {
    $bullet_app->post(function($request) {
        $name = $request->post('name');
        $type = $request->post('type');

        if (!$name || strlen($name) < 3) {
            throw new Exception("Pet's name must be 3 or more characters");
        }
        $res = mysql_query("
            select count(name) from pets where name = '$name'
        ");
        if (mysql_fetch($res)[0][0] > 0)
            throw new Exception("Pet's name already exists");
        }
        if ($type != 'dog' && $type != 'cat' && $type != 'fish') {
            throw new Exception("Pet must be a dog, a cat or a fish");
        }

        // save new pet to the database
    });
});

Yeah, so?

Look, it's running. We shipped! How bad could it really be?

Bad enough

  • Repeatedly duplicated code lends itself to copy/paste errors
  • Gnarly structure and organization leads to comprehension problems
  • Copy/pasted code copies and pastes bugs, too

It's not testable!

Empirical Studies on Quality in Agile Practices (2010)
Fifteen studies show extensive quality improvement directly resulting from TDD.
Realizing quality improvement through test driven development (2008)
Introduction of TDD led to a 40% defect reduction at IBM; 60-90% at Microsoft.
On the Effectiveness of Unit Test Automation at Microsoft (2009)
Defects decreased 21% after introducing automated unit tests.
The Economics of Unit Testing (2006)
Defects found at the unit test level take orders of magnitude less time to find and fix than defects found at the integration test level.

If you can't test your code, you're missing out—and so are your users.

Enough with the noodling, let's get started!

Laying a foundation: Tools

Composer

the package manager for people who hate package managers

  • (Virtually) no dependencies, so no fighting with your sysadmin
  • No side effects outside your project, so no fighting with your sysadmin
  • (Almost) no opinions, so you can add it to any project at any stage
  • Did I mention you won't have to fight with your sysadmin?

$ curl -sS https://getcomposer.org/installer | php
$ ./composer.phar init
$ ./composer.phar install
$ ls
vendor  composer.json   composer.lock

PHPUnit

(Really they should just build it into PHP.)

Failing that, installation with Composer is absurdly straightforward: 

$ ./composer.phar require --dev phpunit/phpunit

PHPUnit Crash Course

$ vi tests/MyTest.php

class MyTest extends \PHPUnit_Framework_TestCase {
    public function testThatMathStillWorks() {
        $this->assertEquals(4, 2 + 2);
    }
}
                        

$ ./vendor/bin/phpunit tests/
PHPUnit 4.3.4 by Sebastian Bergmann.

.

Time: 14 ms, Memory: 2.50Mb

OK (1 test, 1 assertion)

Laying a foundation: Concepts

The Single Responsibility Principle

or

Do one unique thing

Why's it matter?

  • Maintainability
  • Testability!

Spaghetti is responsible for lots of things


<body>
<?
$res = mysql_query("select * from posts order by date desc limit 10");
if (!$res) {
    echo "
Whoops, no posts!
";
} else {
    while ($post = mysql_fetch_assoc($res)) {
?>
        

            
<?= $post['title'] ?>

            
<?= $post['body'] ?>

        

<?
    }
}
?>
</body>

Make your code lazy

(with dependency injection)

What's a dependency?


function getTemplateFile($templateName) {
    $config = new Config(); // <-- this is a dependency!

    return $config->templateDirectory.$templateName.'.html.php';
}

Injecting laziness

When your code has too much responsibility, take some away.


function getTemplateFile($templateName, $c) { // <-- dependency injected!
    return $c->templateDirectory.$templateName.'.html.php';
}

SRP, DI and PHPUnit, oh my!


class Template {
    public function getTemplateFile($templateName, $config) {
        return $config->templateDirectory.$templateName.'.html.php';
    }
}
                        

class TemplateTest extends PHPUnit_Framework_TestCase {
    public function testGetFile() {
        $config = $this->getMock('stdClass');
        $config->templateDirectory = 'foo/';

        $template = new Template();
        $result = $template->getTemplateFile('bar', $config);
        $this->assertEquals('foo/bar.html.php', $result);
    }
}

Guiding principles

1. Don't try to do it all at once.


function getUserById($id) {
    if ($id >= 0) {
        $res = mysql_query("
            select * from users
            where id = $id
        ");
        if ($res) {
            return mysql_fetch_assoc($res)[0];
        }
    }
    return null;
}

function getUserByNameAndPassword($name, $password) {
    if ($name) {
        $res = mysql_query("
            select * from users
            where id = $id and password = '$password'
        ");
        if ($res) {
            return mysql_fetch_assoc($res)[0];
        }
    }
    return null;
}

Hey, those should be a in a User object! Refactor ALL THE THINGS!

Calm down

Yes, they should. Eventually!

One step at a time

Refactoring getUserById


class User {
    public $details;
    protected $_db;

    public function __construct($db, $details) {
        $this->_db = $db;
        $this->details = $details;
    }
    public static function byId($db, $id) { // injecting a database service
        $stmt = $db->prepare('select * from users where id = ?');
        if ($stmt && $stmt->bind_param('i', $id) && $stmt->execute()
            && ($result = $stmt->get_result()) !== false
            && ($user = $result->fetch_assoc()) !== null
        ) {
            $stmt->close();
        } else return null;

        return new self($db, $user);
    }
}

We passed in an external service to talk to the database, which means we can also pass in a fake to test the logic.


public function testUserByIdPrepareFail() {
    $db = $this->getMock('stdClass', ['prepare']);
    $db->expects($this->once())->method('prepare')->willReturn(false);

    $this->assertNull(User::byId($db, 123));
}

Final piece

Original


function getUserById($id) {
    // return the associative array of user details if the user is found
    // return null otherwise
}

Gutted


function getUserById($id) {
    return ($user = User::byId(new mysqli(), $id)) ? $user->details : null;
}
  • Our untestable function is transformed into a wrapper for a testable class
  • The class will be a foundation for future refactoring and development
  • We didn't have to change the getUserById interface!

2. Don't quit your day job

You, too, can refactor and develop at the same time

  • Work in small increments
  • Maintain interface consistency where feasible

function getUserByNameAndPassword($name, $password) {
    if ($name) {
        $res = mysql_query("
            select * from users
            where id = $id and password = '$password'
        ");
        if ($res) {
            return mysql_fetch_assoc($res)[0];
        }
    }

    return null;
}

Mary Manager wants to you make sure inactive users can't log in.


function getUserByNameAndPassword($name, $password) {
    if ($name) {
        $res = mysql_query("
            select * from users
            where id = $id and password = '$password' and active = 1
        ");
        if ($res) {
            return mysql_fetch_assoc($res)[0];
        }
    }

    return null;
}

Or, you could use that shiny new User class...


class User {
    public static function byName($db, $name) {
        // just like byId, but with the name as the identifier
    }

    public login($pass) {
        if ($this->details['password'] !== $pass) {
            throw new Exception("Passwords don't match");
        }

        if ($this->details['active'] !== 1) {
            throw new Exception("User is not active");
        }
    }
}

The tests cover the business logic


/*
 * @expectedException        Exception
 * @expectedExceptionMessage Passwords don't match
 */
public function testUserLoginFailsWithWrongPassword() {
    $user = new User(null, ['password' => 'password']);
    $user->login('wrong password');
}

/*
 * @expectedException        Exception
 * @expectedExceptionMessage User is not active
 */
public function testUserLoginFailsWithInactiveUser() {
    $user = new User(null, ['active' => 0, 'password' => 'password']);
    $user->login('password');
}

...and the final integration should look familiar


function getUserByNameAndPassword($name, $password) {
    // return null if the user is not found
    // return null if the password does not match
    // return null if the user is not active
    // return the associative array of user details otherwise
}

becomes


function getUserByNameAndPassword($name, $password) {
    $user = User::byName(new mysqli(), $name);
    if (!$user) {
        return null;
    }

    try {
        $user->login($password);
    } catch (Exception $e) {
        return null;
    }

    return $user->details;
}

Achievement unlocked: Multitasker

Refactor old code while simultaneously adding a new feature

3. Be sneaky


<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Transitional//EN"
     "http://www.w3.org/TR/xhtml1/DTD/xhtml1-transitional.dtd">
<html>

<?
$res = mysql_query("select title from pages where name = 'index'");
if ($res) {
    $title = mysql_fetch_assoc($res)[0]['title'];
} else {
    $title = 'Default title';
}
?>

<head>
    <?= $title ?>
</head>

This needs a model, a view and a controller! RIGHT NOW!

Does it? Does it really?


class Page {
    protected $db;
    protected $details;

    public function __construct($db, $details = []) {
        $this->db = $db;
        $this->details = $details;
    }

    public static function load($db, $name = '') {
        // load from db by page name
    }

    public function title() {
        return (isset($this->details['title']) && $this->details['title'])
            ? $this->details['title']
            : 'Default title';
    }
}
                        

public function testPageTitleIsDefaultIfMissing() {
    $page = new Page(null);
    $this->assertEquals('Default title', $page->title());
}

public function testPageTitleIsDefaultIfEmpty() {
    $page = new Page(null, ['title' => '']);
    $this->assertEquals('Default title', $page->title());
}

public function testPageTitleIsSet() {
    $page = new Page(null, ['title' => 'my title']);
    $this->assertEquals('my title', $page->title());
}
                        

<?php $page = Page::load(new mysqli(), 'index'); ?>

<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Transitional//EN"
     "http://www.w3.org/TR/xhtml1/DTD/xhtml1-transitional.dtd">
<html>

<head>
    <?= $page->title() ?>
</head>

Prevention is the best medicine

(but it's very hard)

Where does spaghetti come from?

  • It's faster, initially
  • It's easier, initially
  • Stakeholders don't care that the code is unmaintainable, as long as it works

How do I stop it?

  • Care
  • Discipline
  • Time

That's all, folks!

Questions?

joind.in/13727
https://joind.in/13727

References

  • Ellims, M., Bridges, J., & Ince, D. C. (2006). The Economics of Unit Testing. Empirical Software Engineering, 11(1), 5-31. doi:10.1007/s10664-006-5964-9
  • Nagappan, N., Maximilien, E. M., Bhat, T., & Williams, L. (2008). Realizing quality improvement through test driven development: results and experiences of four industrial teams. Empirical Software Engineering, 2008(13), 289-302. Retrieved from http://research.microsoft.com/en-us/groups/ese/nagappan_tdd.pdf
  • Sfetsos, P., & Stamelos, I. (2010). Empirical Studies on Quality in Agile Practices: A Systematic Literature Review. International Conference on the Quality of Information and Communications Technology, 2010. doi:10.1109/QUATIC.2010.17
  • Williams, L., Kudrjavets, G., & Nagappan, N. (2009). On the Effectiveness of Unit Test Automation at Microsoft. International Symposium on Software Reliability Engineering, 2009. Retrieved from http://collaboration.csc.ncsu.edu/laurie/Papers/Unit_testing_cameraReady.pdf