Embedded Files
What is testable code?
What is testable code?
As discussed in other chapters it can be very useful to have unit tests and unit tests should be relatively simple bits of code that test the output of a function by providing it with certain input. Sometimes however it can be very hard to write unit tests to test certain code. The main reason for that is usually that the code is too complex. Another reason could be that the code is too messy and is doing seemingly unrelated tasks within the same function.
Let's look at an example of code that is hard to test:
<?phpnamespace Scuti\Tests;use Scuti\Database\Database;class NotVeryTestable{ private $db; public function __construct(Database $db) { $this->db = $db; } private function doOneThing(): bool { $oneThing = new OneThing($this->db->getMoreData()); $result = $oneThing->doYourThing(); return $result === 'the thing is done'; } private function doAnotherThing(): bool { $anotherThing = new AnotherThing(new OneThing($this->db->getEvenMoreData()), $this->db); $anotherThing->doInsertingDataThing(); return $this->db->getDataInsertedByAnotherThing(); } public function doSomething(bool $condition): bool { if ($condition) { $result = $this->db->getSomeData(); if ($result['abc'] === 'def') { $data = $this->db->getOtherData(); foreach ($data as $record) { if ($record->countColumns() === 21) { return false; } elseif ($record->isDeleted()) { if ($record['xyz'] === 'thisveryspecialvalue') { if($_SERVER['SERVER_ADDR'] === '192.168.1.1') { return $this->doOneThing() || false; } elseif ($_SERVER['SERVER_ADDR'] === '192.168.1.10') { return $this->doOneThing() && false; } else { $i = 1; foreach ($record as $column => $value) { if ($record->countColumns() < 43) { return 42 % $record->countColumns() > 0; } } if ('blue' === $record['sky_color'] && '12:00:00' === $record['time_of_day']) { return rand() > 45656; } } } } } } return $this->doOneThing(); } $result = $this->db->getSomeOtherData(); if ($result['hij'] === '1234') { $values = []; foreach ($result['hij'] as $value) { if ($value > 0) { $values[] = $value; } else { $values[] = 1 / $value; } } return $this->doAnotherThing() || (bool)$values[5]; } elseif ($result['hij'] === '456') { return true; } return $this->doAnotherThing(); }}By looking at the class above we can find some problems that make this code hard to test.
One quite obvious problem is that the function has a high level of complexity. There are quite a few if statements that can be found. Basically each if statement gives us 2 new code paths that can be executed and also 2 new test cases for us to reach 100% test coverage. For loops this presents us with the same or actually in some cases an even bigger problem. There is the case that the loop is executed, the case where the loop is not executed, and depending on the code there might be cases where the code only does something on the 5th iteration (adding at least 2 more paths).
Another problem that makes this class hard to test is that it seems to have more than one purpose. Basically what the class does is based on the value of the $condition parameter of the doSomething() method. If the parameter evaluates to true it executes one code path, but if the parameter evaluates to false it executes a different code path. This increases the number of possible code paths and thus increases the number of test cases we need to write for this one function.
On one of the code paths our code uses the native PHP rand() function to produce a pseudo random number and compares it to some magic number to return the result. Making it difficult for us to predict the result.
The class also has 2 hard dependencies on the classes OneThing and AnotherThing. (Something is considered a "hard" dependency when the class uses the new keyword to instantiate it). These classes should have their own unit tests and we should not test them in our test cases for this class.
So actually our code does not follow some of the SOLID design principles:
- It violates the Single Responsibility principle
- It violates the open/closed principle, as this class is not very open to extension, but instead invites modification
- It violates the Interface Segregation principle by depending on an implementation of the
Databaseclass instead of just the interface - It violates the Dependency Inversion principle by having hard dependencies on
OneThingandAnotherThing
Violating 4 out of 5 of the SOLID principles is a pretty bad score. However for testing the impact is limited to 3 principles: Single Responsibility, Open/Closed, and Dependency Inversion.
Something that is not depicted in our example is inheritance. Imagine that our class extends a base class that in turn also extends a class. Now if a method is overridden to for example do something extra, we have a lot of hidden code that we also need to test (the function should use the parent keyword to call the original function).
It is also possible to override our original method and totally change the logic of that method (and maybe even no longer call the original function on the parent class). In that case usually the Liskov principle is also violated.
Now that we pointed out a lot of flaws in the class above let's look at an example of testable code:
<?phpnamespace Scuti\Tests;use Scuti\Database\DatabaseInterface;use Scuti\Random\RandomInterface;class TestableOneThing{ private $database; private $oneThingFactory; private $random; public function __construct( DatabaseInterface $database, OneThingFactoryInterface $oneThingFactory, RandomInterface $random ){ $this->database = $database; $this->oneThingFactory = $oneThingFactory; $this->random = $random; } private function doOneThing(): bool { $oneThing = $this->oneThingFactory->getOneThing($this->database->getMoreData()); $result = $oneThing->doYourThing(); return $result === 'the thing is done'; } public function doSomething(): bool { $result = $this->database->getSomeData(); if ($result['abc'] === 'def') { $data = $this->database->getOtherData(); foreach ($data as $record) { if ($record->countColumns() === 21) { return false; } elseif ($record->isDeleted()) { if ($record['xyz'] === 'thisveryspecialvalue') { if($_SERVER['SERVER_ADDR'] === '192.168.1.1') { return $this->doOneThing() || false; } elseif ($_SERVER['SERVER_ADDR'] === '192.168.1.10') { return $this->doOneThing() && false; } else { $i = 1; foreach ($record as $column => $value) { if ($record->countColumns() < 43) { return 42 % $record->countColumns() > 0; } } if ('blue' === $record['sky_color'] && '12:00:00' === $record['time_of_day']) { return $this->random->rand() > 45656; } } } } } } return $this->doOneThing(); }}And:
<?phpnamespace Scuti\Tests;use Scuti\Database\DatabaseInterface;class TestableAnotherThing{ private $database; private $oneThingFactory; private $anotherThingFactory; public function __construct( DatabaseInterface $database, OneThingFactoryInterface $oneThingFactory, AnotherThingFactoryInterface $anotherThingFactory ){ $this->database = $database; $this->oneThingFactory = $oneThingFactory; $this->anotherThingFactory = $anotherThingFactory; } private function doAnotherThing(): bool { $anotherThing = $this->anotherThingFactory->getAnotherThing( $this->oneThingFactory->getOneThing($this->database->getEvenMoreData()), $this->database ); $anotherThing->doInsertingDataThing(); return $this->database->getDataInsertedByAnotherThing(); } public function doSomething(): bool { $result = $this->db->getSomeOtherData(); if ($result['hij'] === '1234') { $values = []; foreach ($result['hij'] as $value) { if ($value > 0) { $values[] = $value; } else { $values[] = 1 / $value; } } return $this->doAnotherThing() || (bool)$values[5]; } elseif ($result['hij'] === '456') { return true; } return $this->doAnotherThing(); }}What we can see here is that the class NotVeryTestable has been refactored into two new classes. This separates the responsibilities, making for 2 single responsibility classes.
Because the implementation of doSomething() has been separated the number of possible code paths decreased.
Another thing we see has changed is that before we had hard dependencies on OneThing and AnotherThing, where now the classes depend on factories that can produce these things. The reason that we need a factory instead of just injecting the dependency itself through the constructor is that these classes have constructor arguments.
In this specific case it seems that we would have been able to inject the already initialized classes, but in a lot of cases we cannot inject the initialized objects, because we have to initialize them using some value that we get from some other logic in our class.
By injecting the dependencies we can easily replace them with mock objects and take control over what code paths will be followed.
Our classes no longer depend on implementations, but rather on interfaces. Doing so makes it easy to replace the dependencies with other ones. In the old version we depend on the Database class for example. Let's say that the Database class represents a database connector for MySQL databases. Now if we want to change our application to use PostgreSQL, then we have to modify all the code that depends on the Database class and make it depend on PostgreDatabase.
By depending on DatabaseInterface we no longer have to do such things. We just make both Database and PostgreDatabase implement the interface and our classes will accept either implementation without any modification.
One last thing that changed is that in the old version we were calling rand() an internal PHP function to generate a pseudo random number. In the refactored version we made an interface that defines that implementing classes should have a public rand() function. That way we can easily control the value of the random number by using a mock object based on RandomInterface.
How to write testable code?
How to write testable code?
Basically keep classes and functions as small as possible and follow the SOLID design principles.
Inheritance can also make it difficult to test in some cases. A good rule of thumb to follow when it comes to inheritance is: "Prefer composition over inheritance". However, like always, there are up- and downsides to that approach. For more information please see wikipedia.
Report abuse