TDD на PHP используя Zend Studio - выделяем из калькулятора зависимость и мочим ее

Краткое содержание прошлых серий:
- подружили Zend Studio и Zend Server
- подружили Zend Studio и GitHub
- написали калькулятор по TDD

Калькулятор умеет суммировать числа в разных системах счисления от 2-ричной до 17-ричной. Его можно развивать дальше (умножение, деление, работа с отрицательными числами), но это не так интересно и будет выглядить +/- так же как и в прошлой серии. Сейчас же пойдем другим путем - от заказчика пришло требование-нежданчик. На базе существующего калькулятора необходимо реализовать калькулятор, работающий с римскими числами, при этом старая возможность суммировать в разных системах счисления должна оставаться не тронутой.

Как быть? Вариантов реализаций множество, но как показывает практика ту, которую мы разберем тут называют в самую последнюю очередь (или не называют вообще).

Если присмотреться, то в существубщем калькуляторе уже наблюдается нарушение принципа единой ответственности (SRP или Single Responsibility Principle)

class Calculator {    
    
    private $Base;
    private $Digits = "0123456789ABCDEFG";
    
    public function calculate($expression, $base) {
        $this->Base = $base;

        if ($this->isContainsInvalidNumber($expression)) {
            throw new RuntimeException('Invalid number');
        }
        
        if ($base > strlen($this->Digits) || $base <= 1) {
            throw new RuntimeException('Invalid base');
        }
        
        preg_match_all('/['.$this->Digits.']+/', $expression, $out);
        
        if (count($out[0]) < 2 || substr_count($expression, '+') != 1) {
            throw new RuntimeException('Invalid expression format');
        }        
            
        $sum = $this->hexToInt($out[0][0]) + 
               $this->hexToInt($out[0][1]);    

        return $this->intToHex ($sum);
    }

    private function isContainsInvalidNumber($expression) {
        $is_invalid = false;
        for ($index = 0; $index < strlen($expression); $index++) {
            if ($expression[$index] == '+') {
                continue;
            }
            
            $int = $this->toInt($expression[$index]);
            $is_invalid |= ($int === false) || $int >= $this->Base;                
        }            
        return $is_invalid;        
    }
    
    private function intToHex($int) {
        $result = '';
        $low = $int;
        do {
            $high = $low % $this->Base;
            $low = (int)$low / $this->Base;
            $result = $this->toHex($high).$result;
        } while ($low >= 1);
        
        return $result;
    }

    
    private function toHex($int) {
        return $this->Digits[$int];
    }
    
    private function hexToInt($hex) {        
        $sum = 0;
        for ($index = 0; $index < strlen($hex); $index++) {
            $sum = $this->Base*$sum + $this->toInt(substr($hex, $index, 1));
        }
        return $sum;

    }
    
    private function toInt($hex) {
        if (is_numeric($hex)) return $hex; 
        return strpos($this->Digits, $hex);
    }

}

Сейчас можно углядеть что он работает так - получая на вход два числа, валидирует правильность чисел, после конвертирует их в 10ричную систему, после складывает и конфертирует обратно в исходную систему счисления. конвертирует -> валидирует -> суммирует -> конвертирует. Не знаю как у тебя, но со словом калькулятор у меня больше ассоциируется глагол "суммирует", а вот конвертация и валидация - это что-то несвейственное ему. Кроме того именно конвертация+валидация подвергается обстрелу с новым требованием-нежданчиком от любимого заказчика. А раз уж меняется - инкапсулируй! Это один из ООП-шных советов. Давай проследуем ему.

Благо ТДД позволил нам делать часто рефакторинг и сейчас все очень подготовленно к выносу. Но нельзя писать код, пока не будет написан тест, заставляющий это сделать. Напишем его.

Первым делом я хочу выделить логику конвертации из x-ричной системы счисления в 10-ричную.

[тест]

class ConvertorTest extends PHPUnit_Framework_TestCase {
  
    private $Convertor;
 

    protected function setUp() {
        parent::setUp ();
        $this->Convertor = new Convertor();
    }
 
    protected function tearDown() {
        $this->Convertor = null;
        parent::tearDown ();
    }
 
    public function testShouldConvertHexToInt() { // вот самое интересное
        $actual =  $this->Convertor->decode('1ABC', '16');
        $this->assertEquals(6844, $actual);
    }
  
}

[фикс]

class Convertor {    
    
    private $Base;
    private $Digits = "0123456789ABCDEFG";
    
    public function decode($hex, $base) { // все строки кроме этого метода были перенесены копипастом
        $this->Base = $base;
        
        return $this->hexToInt($hex);
    }
    
    private function hexToInt($hex) {
        $sum = 0;
        for ($index = 0; $index < strlen($hex); $index++) {
            $sum = $this->Base*$sum + $this->toInt(substr($hex, $index, 1));
        }
        return $sum;
    
    }
    
    private function toInt($hex) {
        if (is_numeric($hex)) return $hex;
        return strpos($this->Digits, $hex);
    }
}

[коммит] Начал выносить зависимости по конвертации в отдельный класс
[тест]

    public function testShouldConvertIntToHex() {
        $actual =  $this->Convertor->code('6844', '16');
        $this->assertEquals('1ABC', $actual);
    }

[фикс]

class Convertor {    
    
    private $Base;
    private $Digits = "0123456789ABCDEFG";
    
    public function decode($hex, $base) {
        $this->Base = $base;
        
        return $this->hexToInt($hex);
    }
    
    public function code($hex, $base) { // добавили новый метод
        $this->Base = $base;
    
        return $this->intToHex($hex);
    }
    
    private function intToHex($int) { // это скопитырили из калькулятора
        $result = '';
        $low = $int;
        do {
            $high = $low % $this->Base;
            $low = (int)$low / $this->Base;
            $result = $this->toHex($high).$result;
        } while ($low >= 1);
    
        return $result;
    }
    
    private function toHex($int) {  // это тоже скопитырили из калькулятора
        return $this->Digits[$int];
    }
    
   ... 
}

[коммит] Вынес вторую, операцию кодирование в x-ричную систему Теперь сразу два теста, один положительный а другой отрицательный.
[тест]

    public function testShouldValidateTrueIfValid() {
        $this->assertTrue($this->Convertor->isValid('6844', '16'));
    }
    
    public function testShouldValidateFalseIfInvalid() {
        $this->assertFalse($this->Convertor->isValid('1ABG', '16'));
    }

[фикс]

class Convertor {    
    
    private $Base;
    private $Digits = "0123456789ABCDEFG";
    
    public function decode($hex, $base) {
        $this->Base = $base;
        
        return $this->hexToInt($hex);
    }
    
    public function code($hex, $base) {
        $this->Base = $base;
    
        return $this->intToHex($hex);
    }
    
    public function isValid($hex, $base) { // новый метод
        $this->Base = $base;
    
        return !$this->isContainsInvalidNumber($hex);
    }
    
    ... 
    
    private function isContainsInvalidNumber($expression) { // копитыренный метод
        $is_invalid = false;
        for ($index = 0; $index < strlen($expression); $index++) {
            if ($expression[$index] == '+') {
                continue;
            }
                
            $int = $this->toInt($expression[$index]);
            $is_invalid |= ($int === false) || $int >= $this->Base;
        }
        return $is_invalid;
    }
}

?>

[коммит] Вынес вторую, операцию кодирование в x-ричную систему Я вижу много дублирования, а потому сделаю рефакторинг

class Convertor {    
    
    private $Base;
    private $Digits = "0123456789ABCDEFG";
    
    public function decode($hex, $base) { // тут передается $base
        $this->Base = $base; // а тут устанавливается в поле
        
        return $this->hexToInt($hex);
    }
    
    public function code($hex, $base) { // и тут
        $this->Base = $base; // и тут
    
        return $this->intToHex($hex);
    }
    
    public function isValid($hex, $base) { // и тут
        $this->Base = $base; // и тут
    
        return !$this->isContainsInvalidNumber($hex);
    }

[рефакторинг] избавился от дублирования

class Convertor {    
    
    private $Base;
    private $Digits = "0123456789ABCDEFG";
    
    public function __construct($base) { // появился конструктор принимающий основание
        $this->Base = $base;
    }
    
    public function decode($hex) { // зато основание пропало отсюда
        return $this->hexToInt($hex);
    }
    
    public function code($hex) {
        return $this->intToHex($hex);
    }
    
    public function isValid($hex) {
        return !$this->isContainsInvalidNumber($hex);
    }

Естественно это привело к исправлению тестов.

class ConvertorTest extends PHPUnit_Framework_TestCase {
        
    private $Convertor;
    
    protected function setUp() {
        parent::setUp ();
        $this->Convertor = new Convertor(16); // тут
    
    }
    
    protected function tearDown() {
        $this->Convertor = null;
        parent::tearDown ();
    }
    
    public function testShouldConvertHexToInt() {
        $actual =  $this->Convertor->decode('1ABC'); // тут
        $this->assertEquals(6844, $actual);
    }
    
    public function testShouldConvertIntToHex() {
        $actual =  $this->Convertor->code('6844'); // тут
        $this->assertEquals('1ABC', $actual);
    }
    
    public function testShouldValidateTrueIfValid() {
        $this->assertTrue($this->Convertor->isValid('6844')); // тут
    }
    
    public function testShouldValidateFalseIfInvalid() {
        $this->assertFalse($this->Convertor->isValid('1ABG')); // а тут даже тест немного переделал
    }
    
}

[коммит]
[рефакторинг] Дальше пачка методов ничего не делающих, а только валидирующих

    public function decode($hex) {
        return $this->hexToInt($hex); 
    }
    
    public function code($hex) {
        return $this->intToHex($hex);
    }
    
    public function isValid($hex) {
        return !$this->isContainsInvalidNumber($hex);
    }

Вердикт - убрать, переименовав hexToInt в decode, intToHex в code, isContainsInvalidNumber в isValid с инвертированием return!

class Convertor {    
    
    private $Base;
    private $Digits = "0123456789ABCDEFG";
    
    public function __construct($base) {
        $this->Base = $base;
    }
    
    public function code($int) { // переименовали и сделали public
        $result = '';
        $low = $int;
        do {
            $high = $low % $this->Base;
            $low = (int)$low / $this->Base;
            $result = $this->toHex($high).$result;
        } while ($low >= 1);
    
        return $result;
    }
    
    
    private function toHex($int) {
        return $this->Digits[$int];
    }
    
    public function decode($hex) { // переименовали и сделали public
        $sum = 0;
        for ($index = 0; $index < strlen($hex); $index++) {
            $sum = $this->Base*$sum + $this->toInt(substr($hex, $index, 1));
        }
        return $sum;
    
    }
    
    private function toInt($hex) {
        if (is_numeric($hex)) return $hex;
        return strpos($this->Digits, $hex);
    }
    
    public function isValid($expression) { // переименовали и сделали public
        $is_invalid = false;
        for ($index = 0; $index < strlen($expression); $index++) {
            if ($expression[$index] == '+') {
                continue;
            }
                
            $int = $this->toInt($expression[$index]);
            $is_invalid |= ($int === false) || $int >= $this->Base;
        }
        return !$is_invalid; // инвертировали результат перед возвратом
    }
}

[коммит]
[рефакторинг]Теперь можно подключить акуратно перенесенную реализацию к калькулятору и удалить у него лишние методы.

require_once 'application\models\Convertor.php'; // незабываем импорт

class Calculator {    
    
    // это нам больше тут не понадобится - удаляем
    // private $Base; 
    private $Digits = "0123456789ABCDEFG";
    
    public function calculate($expression, $base) {
        $convertor = new Convertor($base); // создаем новый инстанс конвертера

        if (!$convertor->isValid($expression)) { // валидация (осторожно, инвертируем результат)
            throw new RuntimeException('Invalid number');
        }
        
        if ($base > strlen($this->Digits) || $base <= 1) {
            throw new RuntimeException('Invalid base');
        }
        
        preg_match_all('/['.$this->Digits.']+/', $expression, $out);
        
        if (count($out[0]) < 2 || substr_count($expression, '+') != 1) {
            throw new RuntimeException('Invalid expression format');
        }        
            
        $sum = $convertor->decode($out[0][0]) + // конвертер декодирует, а калькулятор суммирует
               $convertor->decode($out[0][1]);    

        return $convertor->code($sum); // конвертер кодирует обратно
    }

}

[коммит]

Что мы имеем сейчас? Зависимость акуратно шаг за шагом была вынесена в отдельный класс Convertor.

Старые тесты калькулятора тестируют два класса вместе, что не совсем юнит тест. Есть зависимость у калькулятора в виде инстанциирования new Convertor в теле метода calculate. Эту зависимость необходимо разорвать.

Часть тестов из текущего интеграционного (т.к. тестирует два класса в связке) перенести в ConvertorTest, а CalculatorTest переписать на моках.

Успокоения ради можно оставить один интеграционный тест, который проверит два класса в связке.

Но прежде всего хотелось бы сделать метод isValid не знающим про знак суммирования.

Так же стоит еще проверку на base вынести в конструктор Converter - ей там больше место.

После этой операции можно приступать к разработке сумматора римских чисел.

Это мой тест лист на текущий момент. Из него выбираем следующий наиболее простой шаг и вперед!

[рефакторинг] Начну с того, чтобы isValid не знал про суммирование, а валидация основания проходила в конструкторе конвертора.

class Calculator {    
    
//     private $Digits = "0123456789ABCDEFG"; // это лишнее теперь
    
    public function calculate($expression, $base) {
        $convertor = new Convertor($base);

//         if (!$convertor->isValid($expression)) {   // валидацию разместили ниже, после парсинга
//             throw new RuntimeException('Invalid number');
//         }
        
//         if ($base > strlen($this->Digits) || $base <= 1) { // это отнесли в конструктор конвертора
//             throw new RuntimeException('Invalid base');
//         }
        
        preg_match_all('/[0-9A-Z]+/', $expression, $out); // поменяли регекспу - теперь она выкусывает все символьно-цифровое
        
        if (count($out[0]) < 2 || substr_count($expression, '+') != 1) {
            throw new RuntimeException('Invalid expression format');
        }        
        
        if (!$convertor->isValid($out[0][0]) || !$convertor->isValid($out[0][1])) { // добавили новую проверку для операндов
            throw new RuntimeException('Invalid number');
        }
            
        $sum = $convertor->decode($out[0][0]) + 
               $convertor->decode($out[0][1]);    

        return $convertor->code($sum);
    }

}

class Convertor {  // а теперь класс конвертора
    
    private $Base;
    private $Digits = "0123456789ABCDEFG";
    
    public function __construct($base) {
        if ($base > strlen($this->Digits) || $base <= 1) { // добавили проверку
            throw new RuntimeException('Invalid base');
        }
        
        $this->Base = $base;
    }
     
        public function isValid($expression) {
        $is_invalid = false;
        for ($index = 0; $index < strlen($expression); $index++) {
//             if ($expression[$index] == '+') { // это лишнее уже
//                 continue;
//             }
                
            $int = $this->toInt($expression[$index]);
            $is_invalid |= ($int === false) || $int >= $this->Base;
        }
        return !$is_invalid;
    }

    ...

[коммит]
[рефакторинг] Сейчас я бы перенес часть тестов на их законное место. Так как класс калькулятора теперь уже занимается только суммированием, то часто тестов из CalculatorTest должна быть перенесена в ConvertorTest. Данные для тестов конвертора будем брать из рассчета того, что передается в него сейчас при запуске тестов CalculatorTest.

class ConvertorTest extends PHPUnit_Framework_TestCase {
        
    public static function validDataProvider() {
        return array(
                array(10, '2', '2'),
                array(10, '4', '4'),
                array(10, '3', '3'),
                array(10, '7', '7'),
                array(10, '11', '11'),
                array(10, '22', '22'),
                array(10, '33', '33'),
                
                array(16, '1', '1'),
                array(16, '2', '2'),
                array(16, '5', '5'),
                array(16, '6', '6'),
                array(16, '8', '8'),
                array(16, '9', '9'),
                array(16, 'A', '10'),
                array(16, 'B', '11'),
                array(16, 'C', '12'),
                array(16, 'D', '13'),
                array(16, 'E', '14'),
                array(16, 'F', '15'),
                array(16, '11', '17'),
                array(16, '19', '25'),
                array(16, '1B', '27'),
                array(16, '1E', '30'),
                array(16, '1C', '28'),
                array(16, '1D', '29'),
                array(16, '22', '34'),
                array(16, '44', '68'),
                array(16, '99', '153'),
                array(16, '100', '256'),
                array(16, '101', '257'),
                array(16, '132', '306'),
                
                array(17, '2', '2'),
                array(17, '3', '3'),
                array(17, '4', '4'),
                array(17, 'D', '13'),
                array(17, 'F', '15'),
                array(17, 'G', '16'),
                array(17, '10', '17'),
                array(17, '1G', '33'),
                array(17, '1F', '32'),
                array(17, '3F', '66'),
                
                array(2, '0101010101', '341'),
                array(2, '1010101010', '682'),
                array(2, '1111111111', '1023'),
        );
    }
    
    public static function invalidDataProvider() {
        return array(
            array(16, 'G'),
            array(2, '10300'),
            array(4, '1A1'),
            array(16, 'QWE'),
            array(16, 'ASD'),    
        );
    }
    
    /**
     * @dataProvider validDataProvider
     */
    public function testShouldConvertHexToInt($base, $hex, $int) {
        $convertor = new Convertor($base);
        $actual =  $convertor->decode($hex);
        $this->assertEquals($int, $actual);
    }
    
    /**
     * @dataProvider validDataProvider
     */
    public function testShouldConvertIntToHex($base, $hex, $int) {
        $convertor = new Convertor($base);
        $actual =  $convertor->code($int);
        $this->assertEquals($hex, $actual);
    }
    
    /**
     * @dataProvider validDataProvider
     */
    public function testShouldValidateTrueIfValid($base, $hex) {
        $this->validate($base, $hex, true);
    }
    
    /**
     * @dataProvider invalidDataProvider
     */
    public function testShouldValidateFalseIfInvalid($base, $hex) {
        $this->validate($base, $hex, false);
    }
    
    private function validate($base, $hex, $isValid) {
        $convertor = new Convertor($base);
        $actual =  $convertor->isValid($hex);
        $this->assertEquals($isValid, $actual);
    }
    
    /**
     * @expectedException InvalidArgumentException
     */
    public function testShouldExceptionWhenBaseIsMoreThan17() {
        new Convertor(18);
    }
    
    /**
     * @expectedException InvalidArgumentException
     */
    public function testShouldExceptionWhenBaseIsLessThan2() {
        new Convertor(1);
    }
}

а вот что остается от стартого CalculatorTest

class CalculatorUnitTest extends PHPUnit_Framework_TestCase {
        
    private $Calculator;
    
    protected function setUp() {
        parent::setUp ();
        $this->Calculator = new Calculator();
    
    }
    
    protected function tearDown() {
        $this->Calculator = null;
        parent::tearDown ();
    }
    
    public function testShouldSumWhenXPlusY() {
        $this->assertEquals(444, $this->Calculator->calculate('123+321', 10));
    }
    
    /**
     * @expectedException InvalidArgumentException
     */
    public function testShouldExceptionWhenInvalidExpression() {
        $this->Calculator->calculate('1', '10');
    }

    /**
     * @expectedException InvalidArgumentException
     */
    public function testShouldExceptionWhenMoreThanOnePlus() {
        $this->Calculator->calculate('1++3', '10');
    }
        
}

[коммит]
 
Я его назвал CalculatorUnitTest, CalculatorTest оставил как есть (для сравнения, на реально проекте я бы удалил интеграционный). Мне в юнит тест для калькулятора предстоит еще добавить несколько тестов, чтобы покрытие калькулятора было максимальным. Тесты не на состояние а на поведение - мне интересно как общается калькулятор с конвертером, и тут без моков нам не обойтись....

Давай сделаем шажок немножечко побольше (рискнем) и провернем сразу несколько операций. Чтобы продемонстрировать что изменилось, покажу diff с помощью тортилки

1) выделим из Calculator зависимость от Converter (будем ее через конструктор инджектить). А так как $base - свойство Converter то из метода calculate так же убираем $base. 

Это приведет к тому, что нам надо будет исправить интеграционный тест (Calculator + Convertor). Напомню его я держу для сравнения, в реальном мире я бы его удалил где-то сейчас (или чуть раньше).

Это безобразно - снова купа дублирования, потому как сетап уже нам не поможет ($base у разных тестов разный и теперь он задается в конструкторе). Но еще можно заметить, что в некоторых тестах мы явно тестируем Convetror и не место тут этим тестам - в общем как ни крути интеграционный тест чем дальше тем больше путает...

2) А что касается юнит теста, то тут все более менее красиво. Настроили в сетапе мок, 

 Сконфигурировали в тесте моки с помощью методов should...... (я повуделял их для лучшей читабельности теста). И усе!
[коммит] 





Пару линков:
Хорошая статья по мокам.
И репозиторий, если удруг понадобится.


Дальше сделаем сумматор римских чисел. Тут чисстое ТДД, нас на секундочку больше не интересует калькулятор - мы просто переименуем Convertor на HexConvertor, и начнем разрабатывать новый класс RymConvertor. Но это уже в следующий раз....