Dobře testovatelný špatný kód

Přečetl jsem si dokument Guide: Writing Testable Code, který shrnuje postupy používané v Google pro psaní testovatelného kódu, což dokumentu dodává jakýsi punc správnosti. Dokument je zajímavý, za přečtení rozhodně stojí, obsahuje i některé zajímavé myšlenky, s částí názorů ale nesouhlasím. K hlavní myšlence – provázání jednotlivých částí aplikace pomocí Guice – ještě něco napíšu, teď bych se chtěl zaměřit na dva příklady, na kterých je vysvětlen špatně a dobře testovatelný kód:

Výpočet daně z obratu

V dokumentu je ukázka tohoto špatně testovatelného kódu (převedl jsem ji do PHP):

<?php
class SalesTaxCalculator {
    private $taxTable;
    function __construct(TaxTable $taxTable) {
        $this->taxTable = $taxTable;
    }
    function computeSalesTax(User $user, Invoice $invoice) {
        // note that "user" is never used directly
        $address = $user->getAddress();
        $amount = $invoice->getSubTotal();
        return $amount * $this->taxTable->getTaxRate($address);
    }
}
?>

Autoři kódu vyčítají, že nebohý tester musí nejprve vytvořit instance tříd User a Invoice. Dále potom to, že nám API lže, protože i když pracuje s adresou a celkovou částkou na faktuře, tak vyžaduje celého uživatele a celou fakturu. No a nakonec to, že je komponenta špatně znovupoužitelná, protože má velké závislosti. Nabízejí tuto úpravu:

<?php
class SalesTaxCalculator {
    private $taxTable;
    function __construct(TaxTable $taxTable) {
        $this->taxTable = $taxTable;
    }
    // Note that we no longer use User, nor do we dig inside the address.
    function computeSalesTax(Address $address, $amount) {
        return $amount * $this->taxTable->getTaxRate($address);
    }
}
?>

Uvedené výhrady se skutečně vyřešily, ale podle mého názoru došlo k něčemu mnohem horšímu – k porušení zapouzdření. Co je komu do toho, jestli nám pro výpočet stačí adresa uživatele a celková částka na faktuře? Co když se v budoucnu výpočet změní tak, že některé položky budou od daně osvobozeny? To je naprosto běžná věc a metoda by s tím měla počítat. Při „správné“ implementaci budeme muset API funkce změnit a veškerá použití v programu přepsat. Ještě horší řešení by bylo všechna volání $invoice->getSubTotal() zaměnit za $invoice->getTotalForSalesTax().

Co na to říká spoluautor dokumentu? E-mailem jsem poslal dotaz Miškovi s popisem mých výhrad. Odpověděl, že v popsaném případě by změnil metodu na computeSalesTax(Address $address, $amount, $tax) a volající kód upravil tak, aby nevolal computeSalesTax($address, $invoice->getSubTotal()), ale místo toho prošel všechny položky na faktuře a celkovou daň si sám nasčítal. Podle mě to dobře ilustruje hlavní nevýhody „dobře testovatelného kódu“: interní změna, která by měla být ve výhradní zodpovědnosti třídy SalesTaxCalculator, se projeví změnou API a část logiky (sečtení jednotlivých daní) musíme přesunout do volajícího kódu (což může vést k nekonzistenci a chybám).

Předání uživatele se mi zdá taky podivné, i když z jiného důvodu – adresa, na základě které se stanovuje daň, by přece měla být uvedena rovnou na faktuře, ne? Navíc proč jsou autoři nedůslední a i když uvádí, že pro výpočet stačí PSČ, tak upravené metodě předávají celou adresu?

Z pohledu testovatelnosti je druhý kód skutečně lepší, ale zbytek programu tím podle mě značně utrpěl. Zatímco u „špatného“ řešení musela vnitřní logiku znát samotná metoda a test, teď její část musí znát všichni.

Přihlašovací stránka

Druhá ukázka je snad ještě křiklavější. Nejprve špatně testovatelný kód:

<?php
class LoginPage {
    private $client;
    private $request;
    function __construct(RPCClient $client, HttpServletRequest $request) {
        $this->client = $client;
        $this->request = $request;
    }
    function login() {
        $cookie = $this->request->getCookie();
        return $this->client->getAuthenticator()->authenticate($cookie);
    }
}
?>

A teď ten dobře testovatelný:

<?php
class LoginPage {
    private $cookie;
    private $authenticator;
    function __construct($cookie, Authenticator $authenticator) {
        $this->cookie = $cookie;
        $this->authenticator = $authenticator;
    }
    function login() {
        return $this->authenticator->authenticate($this->cookie);
    }
}
?>

Testerovi se skutečně ulevilo – stačí mu předat různé cookies s autentikátorem a má hotovo. Proti přímému předání autentikátoru nic nemám, jeho zjišťování z předaného RPC klienta je skutečně samoúčelné. Ale předání $cookie místo celého $request opět považuji za fatální porušení zapouzdření. Co je komu do toho, jestli pro autentizaci používáme cookie? Co když ji v budoucnu budeme chtít vyměnit třeba za klientský HTTPS certifikát? U „špatného“ přístupu stačí vyměnit vnitřek metody, u „správného“ nás opět čeká přepsání celého programu.

Závěr

Snadno testovatelný kód pro někoho automaticky znamená dobrý kód. Ušetření práce testerům (kteří si musí zjistit, jak se funkce na jednotlivé vstupy musí chovat) ale není automatickou zárukou dobrého programu a někdy může jít úplně proti tomu.

Jakub Vrána, Dobře míněné rady, 22.6.2011, diskuse: 36 (nové: 0)

Diskuse

Diskuse je zrušena z důvodu spamu.

Navigace

PHP triky

• Archiv (704), English articles
• RSS, Atom, Atom diskusí
• O autorovi, o webu, Twitter

Kupte si mou knihu

Reklama

• Hledáte-li programátora nebo naopak sami programujete a nemáte do čeho píchnout, využijte služeb portálu nezávislých profesionálů Na volné noze.

Web běží na serveru

• Váš hosting

Články podle skupin

• Dobře míněné rady (111)
• Řešení problému (170)
• Výuka (100)
• Seznámení s oblastí (102)
• Ze zákulisí (69)
• Školení (31)
• Verze PHP (8)
• Osobní (46)
• Adminer (67)

Výběr článků

• Traverzování kolem stromu prakticky
• Vlastní pořadí řádků v tabulce
• Ukázka použití indexů
• Struktura stránek
• Unikátnost návštěvníka
• Vyhledávání v JavaScriptu
• Kontrola e-mailové adresy
• Vytvoření přátelského URL
• HTTP cachování
• Atomicita operací
• Vypnutí magic_quotes_gpc
• Ukládání souborů od uživatele
• Cross Site Scripting
• AJAX
• Přihlašování uživatelů
• MySQL 4.1 – kódování
• Vzájemné propojení souborů
• Diskuse s reakcemi
• Šablony
• HTTP metody GET a POST
• Escapování
• Ukládání hesel
• Využití unikátních klíčů v databázi
• Obrana proti SQL Injection

Nejnovější články

• Adminer 4.8.1
• Adminer 4.8.0
• Adminer 4.7.9
• Vypnutí chyby při přístupu k nedefinovanému prvku pole
• Users of Adminer 3.7.1 and older might have been hacked

Normy a manuály

• PHP
• MySQL
• HTTP
• Apache
• HTML, XHTML
• JavaScript

Další projekty

• Adminer
• NotORM
• JavaScript Syntax Highlighter
• php-initialized
• Čtyřka