Dobře testovatelný špatný kód

Školení, která pořádám

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

Michal Till:

Myslím si, že tady hraje praktickou roli fakt, že je to psané AFAIK pro typové jazyky. Změna hlavičky, kterou popisuješ, je pak daleko menší bolest. Musím sice přepisovat, ale od kompilátoru nejenže přesně vím kde, ale může mi s tím i pomoct různými refaktoringovými nástroji. Souhlasím, že v PHPku je zrovna tohle dost opruz.

(To není můj celkový názor na věc, ale napadlo mě to jako první jak jsem to viděl.)

David Grudl:

Ten opruz je úplně stejný nezávisle na jazyku, nicméně i kdyby nebyl, není to argument pro. Dle mého jsou ty příklady nedomyšlené. Autor je zřejmě lepší teoretik než účetní :)

Tomas:

Pokud se zmeni autentifikace na HTTPS certifikat neni lepsi pridat novou metodu/implementaci interfacu ktera bude akceptovat jiny parametry?

Ja si myslim ze se snazi vysvetlit ze metoda by mela akceptovat jen nezbytne minimum k tomu aby mohla pracovat. Pokud akceptuje nejaky komplexni objekt jako treba request, tak je potom celkem dost limitovana znovupouzitelnost.

Muj osobni pristup je programovat co nejhloupejsi tridy v tom smylu, ze vstupni parametry neobsahuji celou hromadu dalsich veci ktere by snad nekdy v budoucnu mohly byt potreba - trosku analogie k unixu. Jinak skoncis s tim ze mas classy ktere akceptuji na vstupu zbytecne velice komplexni objekty a oni ve skutecnosti potrebuji k zivotu jen par veci.

Ohledne tech argumentu k prepisovani vnitrni business logiky - tohle treba skoro nikdy nedelam. Trida/implementace je prece navrzena tak aby delalo urcitou specifickou cinnost - jedine upravy ktere se tam potom deji jsou bugfixy. Pokud potrebujes kompletne zmenit vnitrni business logiku protoze chces autentifikovat (nebo cokoliv jineho) jinym zpusobem, tak to spis znamena ze mas udelat zcela novou implementaci (a tu starou samozrejme nechat :)

ikona Augi:

Vzásadě souhlas - (např.) změna autentifikace na HTTPS certifikát by měla znamenat novou implementaci (rozhraní).

Miško IMHO narážel na Law of Demeter (tedy předávat jen to, co se napřímo používá), ale na druhou stranu se to bije s Open/Closed Principle (který nutí používat "obecnější" parametry).

optik:

Ale pánové, k žádnému fatálnímu porušení čehokoli vůbec nedochází. Přijde mi to jako spor "lepší nacpat toho tam více, třeba se to může hodit" kontra "jen to, co je fakt potřeba". To je spíš pocitová věc a nic fatálního a osobně to cítím spíš blíže k druhému.

Z toho paperu mě spíš přijde, že v Googlu došli k tomuto, že v prostředí, kde se testuje, je lepší psát třídy s minimálními závislostmi a případně refaktorovat, prostě praxe a zkušenosti je to takto naučili - tím pádem by byl mimo jak celý tento článek, tak i páně Grudl.

Sebastian:

V podstatě souhlasím, nejspíš jde o dokument vycházející ze zkušeností. V týmech, kde kladou důraz na testovaní je snazší použít kód s minimem závislostí, kontra těm kde jde o čistotu návrhu.

Děkuji Jakube za zajímavý úhel pohledu.

Petr 'PePa' Pavel:

Musím se Mishka zastat, byl jsem na jeho přednášce na ČVUT a nepřišel mi jako teoretik. Měl jsem ale taky dojem, že úleva testerovi je často na úkor něčeho jiného.

Jestli to dobře chápu, je smyslem mít tak snadno testovatelnou aplikaci, že pozdější změna aplikace kvůli přechodu z cookie na certifikát je hračka. Prostě hrábneš do kódu a necháš testy, aby ti samy řekly, kde's udělal chybu. Jestli to tak opravdu funguje v praxi, to netuším.

Jsem taky zastáncem psát aplikaci tak, aby bylo co nejlacinější ji v budoucnu měnit. Myslím, že snaha o co nejhladší testovatelnost jde stejným směrem. Její zastánci věří, že výhody převáží nevýhody.

Taky mi v hlavě zní ještě jedna přednáška na téma bezpečnost, kde usilovali o to, aby jednotlivé komponenty systému (šlo myslím o Javu) měly přístup jen k informacím, které skutečně potřebují. Tj. ne k celému uživateli včetně jeho kreditky, ale jen k PSČ.

Souhlasím s @Tomas.

Denis:

Já obecně raději vidím parametry konstruktorů a metod čím jak nejjednodužší a třídy čím jak "nejtupější". Tzn. přikláním se zde k těm lépe testovatelným variantám. Bez ohledu na nějakou testovatelnost :)

Ono "přepsání celého programu" může ve skutečnosti znamenat to, že se něco někde přepíše jen na 2 místech. A to, že nám kompilér zařve na nějakých místech o kterých jsme už ani nevěděli a tak si alespoň připomeneme zapomenuté části programu.

V praxi se stává, že nějaké metodě se musí sem tam přidat nějaký prametr, nebo nějaký změnit, nebo se musí provést celá refaktorizace. Ale tomu se podle mě nedá nikdy 100% vyhnout. A snažit se cpát do paramtrů zbytečné informace jen proto, že "možná to někdy někdo bude potřebovat" mi připadá zbytečné.

Jen v PHP se mi nepodařilo rozjet debugger, takže tady dostává téma refaktorizace a změna parametrů zcela nový význam... něco jako peklo :D :D

Dále si myslím, že k žádnému porušení zapouzdření nedochází.

Vašek Chromický:

Ale o žádné porušení zapouzdření se nejedná.
Zapouzdření je třeba chápat tak, že nám musí být jedno, jak si to třída LoginPage s tou cookie, o níž si žádá ve svém VEŘEJNÉM rozhraní, uvnitř udělá. Nesmí nám být ale jedno, co že to vlastně bude dělat (což právě každá třída dobrovolně propaguje svým veřejným rozhraním).
Objekt, který se rozhodl, že se stane na třídě LoginPage závislým, si zaslouží (a chce) vědět, že přihlašování probíhá pomocí cookie a ne čehosi jiného. LoginPage bude takovému objektu říkat "pane". Tady žádná skrytá magie není na místě. A až si LoginPage usmyslí, že bude chtít přejít na certifikáty, třeba ji někteří pošlou do háje.

A třída SalesTaxCalculator -- jestliže má počítat daně, tak ať počítá daně, a nestará se o to, že adresu je třeba zjistit z instance nějakého $usera a cenu z $invoice. Třída by tak byla zbytečně závislá na Invoice, User i Address, a přitom by Address bohatě stačila.
Navíc kromě testovatelnosti trpí i její znovupoužitelnost -- kdybych si chtěl adresy i ceny cucat z prstu (nebo číst odjinud), bez Usera se ani nehnu a buď bude pro mě rovnou SalesTaxCalculator nepoužitelná celá, nebo bude nutnost obalování potřebných dat (pokud to tedy půjde) do objektu třídy User zbytečná (špatná) obstrukce navíc.
(Nicméně uznávám, že tohle by mohl být jeden z příkladů, kdy mi v PHPpku chybí přetěžování metod.)

Případá mi, že dle v článku uvedeného principu bychom měli každému objektu nebo každé metodě předávat všechny proměnné, na něž si vzpomeneme, aby v případě, že se rozhodnou změnit své chování, neotravovaly objekty okolo sebe, a my nemuseli nic přepisovat.
V návrhu tak ale vzniká mnoho zbytečných závislostí, klesá znovupoužitelnost, architektura aplikace trpí...

ikona Jakub Vrána OpenID:

Polemiku ohledně zbytečnosti předávání User sis mohl ušetřit – v tomto bodě s autory souhlasím a v článku to jasně uvádím.

Popiš prosím, jak bys upravil kód <?php computeSalesTax($address, $invoice->getSubTotal()) ?> v momentě, kdy by se zjistilo, že některé položky na faktuře jsou od daně osvobozeny. To je totiž ten okamžik, kdy se láme chleba.

Denis:

Jak píšeš, že :"ještě hroší řešení by bylo nahradit $invoice->getSubTotal() za $invoice->getTotalForSalesTax()" Tak to nemusí být pravda.

Pokud by třída computeSalesTax měla místo parametru $amount hned parametr $amountForSalesTax, potom by bylo jasné, že třída chce jen částku, na kterou se aplikuje daň. Každý, kdo by tuto třídu chtěl použít by to věděl. Odkud by tuto hodnotu získával (jestli z třídy $invoice nebo by tam cpal nějakou "konstantu" při testování by bylo jedno).

Dále se dá uvažovat nad tím, že se může přidat parametr do metody $invoice->getSubTotal($OnlyForSalesTax = true) a podle toho upravit tělo této metody. Tím pádem se všude změní chování bez rizika. A když potom budeme chtít "původní" chování, tak zavoláme jen $invoice->getSubTotal(false).

ikona Jakub Vrána OpenID:

Jaká chyba kompilátoru nebo alespoň jaká běhová chyba tě potom upozorní na to, když někde omylem místo <?php computeSalesTax($address, $invoice->getSubTotal()) ?> zavoláš <?php computeSalesTax($address, $invoice->getSubTotal(false)) ?>. Jak budeš všechna tato místa v programu odhalovat?

Denis:

Neupozorní mě na to žádná chyba a ani to není cílem. Cílem bylo jen ukázat další řešení (byť v tomto konkrétním případě bych se k němu nepřiklonil). Pokud již mám program napsaný a najednou potřebuji udělat onu změnu, potom lze přidat ten parametr $OnlyForSalesTax = true. Tzn. po této úpravě program opět funguje dobře.

Pokud poté někdo bude něco dále programovat, tak musí vědět, co kde má napsat. Před blbcem programátorem mě neochrání nic, jelikož logické chyby se těžko někde chytají :)

Těchto "Co když někdo omylem" může být v programu milion a nejde to nijak eliminovat. Od toho jsou programátoři, dobře pojmenované identifikátory, logicky postavený programový kód a samozřejme testy.

ikona Jakub Vrána OpenID:

Napiš tedy řešení, ke kterému by ses přiklonil.

Dobrý návrh spočívá mimo jiné v tom, že blbce programátora upozorní na to, že udělal chybu. Chyby totiž dělá každý a cílem je se o nich dozvědět co nejdřív a s co nejmenšími následky.

Třeba zrovna tahle chyba jde eliminovat triviálně – stačí, aby computeSalesTax() přijímala celou Invoice a sama si určila částku, ze které má daň počítat. Pak se nemůže stát, že nějaký „blbec programátor“ omylem předá jinou částku, než kterou metoda očekává. Stejně tak se nemůže stát, že v případě změny chování zapomenu opravit okolní kód. Proto jsem mluvil o porušení zapouzdření – okolní kód v tvém řešení musí vědět, z čeho se daň počítá, i když to by mělo být výlučnou zodpovědností třídy SalesTaxCalculator.

Denis:

Kód, ke kterému se přikláním já, je:
<?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, $amountForSalesTax) {
        return $amount * $this->taxTable->getTaxRate($address);
    }

    function computeSalesTax(Address $address, Invoice $invoice) {
       amountForSalesTax = //... zjistovani spravne hodnoty z $invoice
       return $this->computeSalesTax($address, amountForSalesTax);
    }
}
?>

"zjistovani spravne hodnoty z $invoice" ovšem nemůže vypadat takto: <?php $invoice->getTotalForSalesTax()?> jelikož Invoice v tomto případě nesmí vědět, jak se správná hodnota počítá.

Dokážu si ale představit i řešení, že to, z čeho se počítá daň, bude vědět právě jen oběkt Invoice a tuto hodnotu bude poskytovat okolí. Protože mi zase příjde dost divné, aby se třída SalesTaxCalculator "hrabala" v položkách faktury třídy Invoice a zjišťovala si, co je předmětem daně a co ne.

Denis:

Moje řešení vyžaduje overloading, takže nechť se aplikuje jen tam, kde je overloading možno použít :)

ikona Jakub Vrána OpenID:

Geniální řešení přebírající to špatné z obou variant – kód je špatně testovatelný (protože druhou metodu při testování samozřejmě nemůžeme vynechat) a zároveň náchylný k chybám (protože nikdo mi nezakáže zavolat první metodu se špatnou částkou).

Shodneme se na tom, že vnitřek computeSalesTax() nemůže volat $invoice->getTotalForSalesTax(). Neshodneme se na tom, že by Invoice věděla, z čeho se počítá daň (když už existuje třída SalesTaxCalculator, tak je to podle mě výlučně její zodpovědnost).

ikona v6ak:

Co se týče testování, podíval bych se na to z jiného úhlu pohledu. Já bych necpal obě metody do stejné třídy, ale spíše bych to udělal jako obálku. Teoreticky by to v jednodušších případech mohlo být jedno, ale lépe je na tom vidět to, co chci ukázat:
Ta druhá třída je něčím, co by se stejně muselo napsat do jedné z těchto vrstev. Takže, pokud ji nebudu testovat samostatně a unit testy budou vypadat trošku jako integrační, protože test bude přes více třída tím i přes více vrstev. Ale kvalita testů nebude horší než kdyby ta vrstva byla sloučená, jen to tak opticky vypadá.
Možnost zadat chybná data je tu trošku schovaná, protože se jen tak programátor nemůže dostat k té vnitřní instanci (musí ji získat jinudy), ale tady bych si prostě dovolil předpokládat, že programátor ví, co dělá, toto se těžko může stát překlepem apod., zvlášť u ztíženého přístupu k obalené instanci.

ikona v6ak:

Proč dávat obě metody do stejné třídy? OOP je skládačka a tady bych to spíše rozdělil do dvou tříd.

paranoiq:

a jak by se situace změnila, kdyby byly dvě různé sazby daně? (u DPH běžná věc)

ikona Jakub Vrána OpenID:

Přesně tuto variantu jsem před chvílí nadhodil Miškovi (spoluautor rozebíraného dokumentu). V téhle diskusi jsem si to chtěl ještě chvíli ušetřit, aby mi zbyl trumf na konec :-). Bohužel nejsem jediný, koho to napadlo…

Denis:

Ale to není žádný trumf :) To je pouze analýza na jejiž základě se potom vytváří třídy. Takže pokud budeme mít několik základů daně a ještě různě uplatnitelné v rúzných zemích a na různých položkách, potom samozřejmě nemůže metoda computeSalesTax přebírat pouze jedno číslo. Tady v tomto případě již bude asi nejvhodnější opravdu předávat jako parametr objekt Invoice, jak navrhuješ.

Jenže toto by určitě schválil i autor článku, jelikož zde jsou všechny předávané parametry nutné i v době testování a testera to tak nenutí vytvářet zbytečně objekt Invoice jen proto, aby z něj metoda comuteSalesTax využila pouze $amount = $invoice->getSubTotal(); Objekt Invoice tester hold musí vytvořit, jelikož je to nutné a logické.

Asi těžko budu mít metodu Resize() pro změnu velikosti obrázku s nějakým parametrem, kde by tester mohl jen předat nějakou jednoduchou hodnotu. Je jasné, že otestování metody Resize() se nevyhne vytvoření nějaké pixelové mapy :)

Jenže toto nebylo cílem článku :)

ikona Jakub Vrána OpenID:

Situace je takováto: vytvořili jsme aplikaci pro stát s jednotnou sales tax a v něm nám perfektně funguje. Aplikace je úspěšná, takže s ní chceme expandovat i do ostatních států. Nejprve do států, kde jsou od daně některé položky osvobozené, potom i do států, kde jsou na různé položky různé sazby daně.

Takže to nemusí být součástí nějaké původní analýzy – první verzi aplikace jsme v souladu s dokumentem vytvořili tak, aby byla co nejlépe testovatelná – tedy s metodou computeSalesTax(Address $address, $amount). A mě zajímá, jak ji upravíme při expanzi do států s některými položkami od daně osvobozenými – ty navrhuješ předat jí $invoice->getSubTotal(true) (chybám nezabráníš, sémantika tiše trpí). A pak mě zajímá, jak ji upravíš při expanzi do států s různými sazbami daně – teď už připouštíš předání celého objektu Invoice.

A příčinou těchto problémů, možných chyb a neustálých změn různých částí programu je porušení zapouzdření – výklad zákona o sales tax nebyl v původní implementaci computeSalesTax(Address $address, $amount) lokalizován ve třídě SalesTaxCalculator, ale jeho důležitá část (z čeho se daň vlastně počítá) byla vyčleněná do volajícího kódu.

Denis:

Když si to celé postavíš do světla tak, jak se ti to momentálně hodí, tak potom s tebou musím souhlasit.

Pokud by situace byla tak, jak píšeš, potom by se ve finále ukázalo, že přístup computeSalesTax(Address $address, $amount); byl podceněný.

$invoice->getSubTotal(true) jsem dal jako ukázku, ale semanticky je to špatně. Správně by mělo být getSubTotal($OnlyForSalesTax = false), což se ale nehodilo :D

Ale tady nejde jen o tu jednu metodu. Tady jse o celkový přístup. A pokud je celý program prorostlý takovými závislostmi, pak se prostě blbě testuje a třídy jsou špatně znovupoužitelné. Vytváří se něco, co komplikuje práci a třeba to nikdy nikdo nebude potřebovat, protože se aplikace prostě rozšiřovat nebude.

Mě osobně zvedá ze židle, když nějaká metoda chce jako parametr nejakou třídu a přitom vím, že stejně ta metoda z té třídy použije třeba jen jeden parmetr. A pak ocházím na přestávku, když teda tu třídu vytvořím... ale... v konstruktoru tato třída chce jinou třídu a já zase vím, že je ta třída v konstruktoru naprosto zbytečně, že by stačily dva číselné parametry.

ikona Jakub Vrána OpenID:

Článkem jsem chtěl ukázat, že ušetření práce testerovi by neměla být při vývoji ta hlavní priorita. Za mnohem důležitější považuji dodržení zapouzdření. A to z ryze praktických důvodů – vede to k jednodušeji upravitelným programům s méně chybami.

Metoda požadující celý objekt tě zvedat ze židle sice může, ale článkem jsem chtěl ukázat, že to někdy může být ku prospěchu celého programu. Ono okolnímu kódu se taky obvykle snadněji volá f($a) než f($a->x, $a->y) a jediný, kdo s tím má problém, je tester, který si to $a holt musí vytvořit, i když by pro něj bylo jednodušší zavolat jen f($x, $y).

Zkrátka rozhodnutí o tom, co má metoda přijímat, by podle mě nemělo vycházet z toho, jak snadno se to bude testovat, ale v čem spočívá vytvářená funkčnost. A určení toho, z čeho se sales tax vlastně počítá, je v uvedeném příkladě nedílnou součástí této funkčnosti.

Denis:

Naprosto souhlasím s tím, že programovat něco nějak jen pro to, aby se to testerovi dobře testovalo, je nesmysl. Naštěstí nejsem tester :D
V mém případě jsem přebíral projekty, které byly místy psány právě v duchu předávání objektů jako parametry. Bylo to zcela zbytečně megalománské právě z důvodu "co když možná někdy", místy až samoúčelné a zpomalovalo to celou aplikaci. Refaktorizace probíhala tak, že jsem tyto parametry nahrazoval jendodužšími.

A když se projekt rozrůstá, tak se mi jeví jako mnohem lepší upravovat po někom kód metody, která přebírá jako parametr dva inegery než když přebírá nějaký šílený objekt. Protože když někde předávám objekt, tak musím zkontrolovat, co ti hoši s tím objektem v té metodě dělají, jestli mi ho tam nějak "omylem" nemění atd. Protože daný objekt samozřejmě má větší životní cyklus než jen vstoupit do jedné metody a pak zemřít :)

Toto jsou ale postřehy z mé praxe, které mohou být jinde jiné. Již za krátko se uvidí (pokud se něco nezměnilo), jak to mají ve Facebooku :)

ikona v6ak:

Ona se hodí určitá neměnnost objektů - ideálně co největší. (I proto dávám přednost konstruktorové DI před setterovou.) Nejsem striktní, abych tvrdil 'Každá třída, která může měnit svůj stav, je prasárna.'. Nicméně, často stojí za zvážení, jestli ta třída musí opravdu být mutable.

Martin Prokeš:

Líbí se mi kulturní rozdíly.

Pan Vrána píš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."

Jsme na tom opravdu špatně, když za "běžnou věc" považujeme změny daní každý rok a tisíce výjimek, které jsou daněny úplně jinak.

Přitom v USA jim tyhle nesmysly vůbec na mysl nepřijdou. Oni zase za "běžnou věc" považují to, že na součet se aplikuje sales tax a hotovo. A vše je stejné desítky let. Žádné poplatky OSA, žádné recyklační poplatky, žádná DPH (navíc tři různé sazby), žádné spotřební a jiné vymyšlené daně. Když tamní startupisté vidí naše daňové předpisy a navíc nemalé platby za účetního (protože vést podvojné účetnictví firmy není nic pro laiky), tak se jim vždy dělá špatně.

ikona Jakub Vrána OpenID:

S těmi kulturními rozdíly je to asi takhle: “Nearly all jurisdictions provide numerous categories of goods and services that are exempt from sales tax, or taxed at reduced rates.” http://en.wikipedia.org/wiki/Sales_taxes_in_the_United_States

paranoiq:

v USA se daň z obratu liší v každém státě a leckde i v každém okrese. není jen jedna sazba, ale několik (stejně jako u naší DPH). některé sazby jsou uplatňovány až od určité výše. výjimek jsou desítky. v některých státech daň z obratu vůbec nemají

toliko k rozdílům

ikona v6ak:

Jsem dalek toho, abych vyjádřil jednoznačný názor na to, co je lepší. Oboje má svoje výhody i nevýhody.
* Pokud třída něco počítá, může toho chttít co nejvíce, aby mohla v budoucnu počítat i na základě většího množství informací.
* Pokud po třídě něco chceme, budeme mít tendenci jí toho dávat co nejméně, abychom v budoucnu ji mohli použít i jinde.
Je asi jasné, že jde o dva navzájem protichůdné požadavky. Ať zvolím jakkoli, nebudu mít záruku znovupoužitelnosti. Tady je na místě určitá dávka pragmatismu a vybrat si podle aktuálně řešeného problému. V případě cookies je ale třeba zřejmé, že je mimo HTTP potřebovat nebudu.

Je tu ještě jedna alternativní možnost: implementovat to jako mezivrstvu. V případě autentozace bychom měli nějaký sušenkový autentizátor a k němu požadavkový autentizátor, který by byl wrapperem sušenkového autentizátoru. Dokud bych používal zvenku jen požadavkový autentizátor, mohl bych jej celkem snadno vyměnit dost možná na jednom místě. Jakmile bychom začali používat cookies mimo HTTP, pořád můžeme, ale zkomplikuje nám to změnu. Ano, to je už trošku extrém (psal jsem, že cookies asi mimo HTTP nevyužijeme), ale obecně je to jedna z možných cest, která se může v některých případech hodit. Ač to možná není na první pohled zřejmé, tu mezivrstvu v kódu budeme mít vždy, jen je otázka, zda ji oddělíme, nebo ji přilepíme k některé z vrstev.

Dundee:

To už bychom mohli rovnou předávat všude ServiceLocator :)

Petr 'PePa' Pavel:

Ještě poznámka: tester není nepřítel :-) Tester bys měl být (i) ty sám, takže se nezdráhej mu ušetřit práci. Mishko taky říkal, že by testy měly vypadat jako "pohádka", stručné, přehledné, s jasným poselstvím. Když je před každým assert hromada vytváření objektů, tak se myšlenka testu ztrácí a riskuješ chybu v testu. Chyba v testu je taky špatně, protože takový test ti nepomůže odhalit chybu v programu. Takže my chceme testerovi ulehčovat práci.

ikona Ondřej Mirtes:

Já stále vidím rovnítko mezi dobrou testovatelností a dobrou použitelností. Vždyť v produkčním kódu s objekty pracuji stejně jako v testech, tak při jejich sestavování využiji pohodlí na obou místech.

ikona Jarda Jirava:

Ahoj, mě se na tom nelíbí to, že se vstupní metoda snaží jakýmkoliv způsobem předvídat, co bude uvnitř sebe potřebovat. Budu uvažovat jen první případ, kdy je potřeba vypočítat daň pro fakturu.
Podle mě by vstupem do metody měly být jenom položky faktury a nic více. Kalkulátor jako takový by pak měl přebírat strategii, pomocí které bude vypočítávat daň z jedné položky. Tato strategie budiž pak vybrána (zkonstruována) na základě dalších údajů, které si však nebude řídit Kalkulátor, ale nějaká factory, která bude zodpovědná za předání správné strategie.
Kalkulátor pak nad každou položkou faktury zavolá metodu strategie a výsledek nejspíše sečte - opět je otázkou, zda by samotný kalkulátor celkové daně neměl být jedním z typů strategie, jak položky sčítat (co kdyby se našla taková země, že daň je určována jen podle nejvyšší hodnoty daně).
Z tohoto pohledu, pak budu samostatně testovat, zda jsem vybral správnou strategii, zda správně počítám daň pro jednu položku a zda dostanu správnou celkovou částku.
Samozřejmě těch možností, jak toto celé konstruovat by bylo více.
-- J.

ikona Jakub Vrána OpenID:

E-mailem jsem požádal Miška (spoluautor citovaného dokumentu) o vyjádření: http://php.vrana.cz/dobre-testovatelny-spatny-kod.php#misko.

Diskuse je zrušena z důvodu spamu.

avatar © 2005-2024 Jakub Vrána. Publikované texty můžete přetiskovat pouze se svolením autora. Ukázky kódu smíte používat s uvedením autora a URL tohoto webu bez dalších omezení Creative Commons. Můžeme si tykat. Skripty předpokládají nastavení: magic_quotes_gpc=Off, magic_quotes_runtime=Off, error_reporting=E_ALL & ~E_NOTICE a očekávají předchozí zavolání mysql_set_charset. Skripty by měly být funkční v PHP >= 4.3 a PHP >= 5.0.