Průběžný refaktoring

27. 11. 2016

Kód není nikdy dokonalý, ať se snažím sebevíc. Určitě ale není dobré kód ponechávat v tom nejranějším stádiu. Pokud s kódem skončím hned jak dosáhnu požadované funkčnosti, pravděpodobně se to časem vrátí. Je to dluh. Technický dluh. Čím více aplikace poroste, čím častěji se budou měnit požadavky, čím starší bude, tím bude každá další práce dražší a dražší. Jestliže s tím nebudu nic dělat, dojde to do stavu, kdy implementace sebemenší drobnosti zabere neúměrné množství času. Když kód začne fungovat, ještě to neznamená, že je napsaný dobře.


Nebudu zde popisovat jak psát dobrý kód a že je potřeba psát testy. Chci se zaměřit čistě jen na to, jak po sobě nezanechat kód, který někomu jinému přidělá pěkných pár vrásek na čele a jak toto obhájit u managementu. Ten někdo jiný může být nový kolega, ale i já sám za několik let. Mimo těch vrásek jde taky o zbytečně vynaložený čas a tedy i peníze.

Co ušetřím teď tím, že po sobě nezametu, vrátí se později i s pěkně tučným úrokem.

Jde mi o to obhájit, že refaktoring je potřeba. A že to není zbytečná práce navíc, ale součást naší běžné práce.

Co je a co není refaktoring

Refaktoring je zjednodušeně změna stávajícího kódu beze změny jeho funkčnosti [wiki].

Nikdy nerefaktoruji kód a zároveň měním (přidávám, upravuji, odebírám) funkčnost. Nejdříve refaktoruji a až pak měním funkčnost nebo obráceně. Pokud bych měnil funkčnost, už to není refaktoring.

Jednoduché zadání s průběžnými změnami

Zkusím to celé ukázat na jednoduchém příkladu. Jde o jednoduchou část Nette aplikace s Latte šablonami. Pokud Nette framework neovládáte, nevadí. Zaměřím se jen na podstatný kód, zbytek vynechám.

Není nutné zcela chápat veškerý kód, ale způsob a postupy, jak řešit požadavky a nebýt prasátečko.

Kód refaktoruji, když mi to dává smysl. To je tehdy, když jsem něco napsal a není to přehledné, je to napsané zbytečně složitě. Pak refaktoruji hned co takový kód napíši. Také refaktoruji když mám provést někde nějakou změnu a kód, kterého by se ta změna týkala, je ošklivý. Změna by se mi dělala příliš obtížně nebo by nebyla možná vůbec. Pak takový kód refaktoruji aby umožňoval co potřebuij a následně napíši požadované změny.

Toto zkusím ukázat na příkladu kdy něco implementujeme a klient neustále přidává nebo mění požadavky.

"Zobrazujme článek jako prostý text!"

Velmi jednoduché zadání. Představme si, že články už mám nějak implementované. Že mám nějaký model, presenter/controller a šablonu.

V šabloně mám článek v proměnné $article a z něj si vytáhnu obsah metodou ::getContent().

Implementuji tedy pouze šablonu, ta může vypadat nějak takto:

<html>
    {$article->getContent()}
</html>

A mám hotovo.

"Chci to formátovat a chci to psát jako HTML!"

Řekněme, že článek zadává pouze a jen administrátor, a že klientovi nedokážeme vysvětlit, že to HTML je zbytečné.

Zkrátka, v administraci se spravuje článek přes nějaký HTML editor a na frontendu jej budeme zobrazovat jako HTML.

Nejjednodušeji to mohu v šabloně udělat takto:

<html>
    {$article->getContent()|noescape}
</html>

Mám tedy další požadavek implementovaný.

"On by měl být ten vstup bezpečný, ale chceme pořád HTML!"

Domluvili jsme se, že je potřeba odstraňovat JavaScript - ten je nežádoucí, admin může být zlý, je ve výpovědní lhůtě a chce škodit. Navíc se může stát, že vstup některého článku nebude od administrátora a máme tu XSS.

Klient si ale nadále trvá na HTML a stále se nám jej nedaří přesvědčit.

Upravím si tedy generování výstupu do šablony tak, abych mohl text před zobrazením nějak zpracovat.

Přidám si makro {html} za kterým bude nějaký magický regulární výraz (třeba).

Stále jen implementuji nové požadavky a ještě jsem stále nic nerefaktoroval.

<?php

namespace App\Front\Presenters;

use Latte;

class ArticlePresenter extends BasePresenter
{

    // ...

    protected function createTemplate()
    {
        $template = parent::createTemplate();
        $latte = $template->getLatte();
        $set = new Latte\Macros\MacroSet($latte->getCompiler());

        $set->addMacro('html', function (Latte\MacroNode $node, Latte\PhpWriter $writer) {
            return $writer->write('echo preg_replace(..., ..., %node.args);');
        });

        return $template;
    }

}

A změním šablonu:

<html>
    {html $article->getContent()}
</html>

Ale takové řešení není úplně přehledné (php echo, escapování apostrofů a ještě ve vnořeném regulárním výrazu, Latte placeholdery jako %node.args). A taky tím zbytečně komplikuji jakoukoliv další práci. Třeba přidání dalších modifikací do stejného makra.

Implementoval jsem tedy další požadavky, ale začíná to celé kynout. Měl bych tedy nyní už refaktorovat.


protected function createTemplate() { $template = parent::createTemplate(); $latte = $template->getLatte(); $set = new Latte\Macros\MacroSet($latte->getCompiler()); $set->addMacro('html', function (Latte\MacroNode $node, Latte\PhpWriter $writer) { return $writer->write('echo call_user_func($this->global->sanitizeHtml, %node.args);'); }); $latte->addProvider('sanitizeHtml', function (string $s): string { return preg_replace(..., ..., $s); }); return $template; }

Kód je teď sice delší, ale mám oddělené samotné Latte makro a funkci pro sanitizaci HTML (která magicky odstraňuje ten JavaScript).

Nestálo to skoro žádný čas navíc. Dalšímu člověku, co bude s touto funkcí pracovat, jsem ulehčil život. Nemusí řešit a znát, jak implementovat Latte makro.

Aby to bylo ještě zřetelnější, mohl bych extrahovat anonymní funkci do metody (ještě refaktorovat), takto:


protected function createTemplate() { $template = parent::createTemplate(); $latte = $template->getLatte(); $set = new Latte\Macros\MacroSet($latte->getCompiler()); $set->addMacro('html', function (Latte\MacroNode $node, Latte\PhpWriter $writer) { return $writer->write('echo call_user_func($this->global->sanitizeHtml, %node.args);'); }); $latte->addProvider('sanitizeHtml', [$this, 'sanitizeHtml']); return $template; } public function sanitizeHtml(string $s): string { return preg_replace(..., ..., $s); }

Když se sem k tomu vrátím za 2 roky a už třeba nebudu vyvíjet s Nette (nebudu schopen řešit implementaci Latte makra) nebo sem přijde člověk co Latte nezná, nevadí.

Celou metodu ::createTemplate můžu ignorovat. A řeším jen primitivní ::sanitizeHtml.

Tam už zvládne úpravy nebo přidání dalších věcí (téměř) kdokoliv. Podívejme na ni:


public function sanitizeHtml(string $s): string { return preg_replace(..., ..., $s); }

Stálo to jen minimum práce navíc, ale dokáže s tím teď pracovat i člověk bez znalosti Nette/Latte.

"Přeci jen to HTML vlastně nepotřebujeme..."

Klienta se povedlo přesvědčit, že nepotřebuje zbytečně robustní HTML editor s miliony pozlátek a hordou potíží, ale že mu stačí jen jednoduše formátovat text.

Nemusím pak už řešit které HTML tagy povolit a které ne, XSS, spoléhat se na to, že vstup je opravdu jen od administrátora. A taky se nebude rozbíjet layout.

Domluvili jsme se na Markdown syntaxi (která je jednoduchá) a aby s tím mohl pracovat naprosto kdokoliv, v administraci jsme HTML editor nahradili za Markdown editor. To zde řešit nebudu, soustřeďme se na samotné zobrazení Markdown na frontendu.

Musím převést Markdown na HTML. Ten samotný převodník psát nebudu, použiji michelf/php-markdown. Není ale příliš podstatné - řešíme refaktoring.

Mám už makro {html} a tohle bude skoro stejné. Uvidíme, jestli jsem kód po zapracovávání požadavků refaktoroval aby nebyl příliš složitý. Makro {html} již nepotřebuji, tak jej nahradím. Celé by to mělo nyní jít i bez znalosti Latte API.

Zkusím nyní zapracovat další požadované změny (nerefaktoruji, implementuji další požadavky). Napíšu si nové makro {markdown}:


protected function createTemplate() { // ... $set->addMacro('markdown', function (Latte\MacroNode $node, Latte\PhpWriter $writer) { return $writer->write('echo call_user_func($this->global->markdownToHtml, %node.args);'); }); $latte->addProvider('markdownToHtml', [$this, 'markdownToHtml']); // ... } /** * Converts Markdown syntax into HTML. * @see https://github.com/michelf/php-markdown */ public function markdownToHtml(string $s): string { $md = new \Michelf\MarkdownExtra(); return $md->transform($s); }

A kód, co byl potřeba pro {html} makro, odstraním.

Má smysl po téhle implementaci zase něco refaktorovat?

Jak jsem psal, kód není nikdy dokonalý. Jen se musím pokusit najít vhodnou rovnováhu mezi přijatelnou kvalitou a rozpočtem..

Nechal bych to takhle.

"Chceme v HTML u každého nadpisu ID!"

Dobře, můžu to přidat snadno. Stačí k tomu dokumentace použité knihovny co převádí Markdown do HTML.

Řeší to callback v property \Michelf\MarkdownExtra::$header_id_func. To nemusím rozebírat, řešíme refaktoring.

Upravím funkci/metodu která se volá v makru markdown (nerefaktoruji, implementuji):


public function markdownToHtml(string $s): string { $md = new \Michelf\MarkdownExtra(); $md->header_id_func = function (string $s): string { return 'toc-' . \Nette\Utils\Strings::webalize($header); } return $md->transform($s); }

A mám to. Je to řešení vhodné? Uvidíme.

"Chceme taky syntax highlighting!"

Dobře, to použitá knihovna pro Markdown taky umožňuje.

Řeší to callback v property \Michelf\MarkdownExtra::$code_block_content_func.

Kód, co zvýrazňuje syntaxi různých jazyků jako HTML, psát nebudu. Použiji kukulich/fshl. Toto je ale opět nepodstatné více rozebírat, jde jen o princip.

Funkcionalitu přidám podobně jako když jsem měl generovat ID u každého nadpisu.

Zase jen implementuji další požadavky, nerefaktoruji.


public function markdownToHtml(string $s): string { // ... $md->code_block_content_func = function (string $code, string $class): string { $lexerClass = 'FSHL\Lexer\\' . ucfirst($class); if (class_exists($lexerClass)) { $fshl = new \FSHL\Highlighter(new \FSHL\Output\Html, \FSHL\Highlighter::OPTION_TAB_INDENT); $highlighted = $fshl->highlight($code, new $lexerClass); } else { $highlighted = htmlspecialchars($code, ENT_NOQUOTES); } return $highlighted; } // ... }

Nechám to takhle být?

Řekl bych, že to nějak bobtná. A že toho presenter/controller řeší zbytečně moc. Tohle tam nepatří. Když bych takhle pokračoval, můžu napsat celou aplikaci do jedné třídy. A taky se vykašlat na nějaké MVC. A pak se z toho posadit na zadek a narazit si kostrč.

Jdu to trochu refaktorovat. Extrahuji implementaci toho převodu Markdown -> HTML do samostatné třídy:

<?php

namespace Project\Markdown;

use FSHL;
use Michelf\MarkdownExtra;
use Nette\Utils\Strings;

class Markdown
{

    public function transform(string $text): string
    {
        return $this->createMDE()->transform($text);
    }

    public function highlight(string $code, string $class): string
    {
        $lexerClass = 'FSHL\Lexer\\' . ucfirst($class);
        if (class_exists($lexerClass)) {
            $fshl = new FSHL\Highlighter(new FSHL\Output\Html, FSHL\Highlighter::OPTION_TAB_INDENT);
            $highlighted = $fshl->highlight($code, new $lexerClass);
        } else {
            $highlighted = htmlspecialchars($code, ENT_NOQUOTES);
        }

        return $highlighted;
    }

    public function toc(string $header): string
    {
        return 'toc-' . Strings::webalize($header);
    }

    public function createMDE(): MarkdownExtra
    {
        $parser = new MarkdownExtra();

        $parser->header_id_func = [$this, 'toc'];
        $parser->code_block_content_func = [$this, 'highlight'];

        return $parser;
    }

}

Tím jsem celou tuhle záležitost zabalil dohromady. A v presenenteru už nebudu pracovat s X různými knihovnami (navíc třetích stran - nemám plně pod kontrolou).

Refaktoruji ještě presenter aby používal extrahovanou třídu. Bude ve výsledku vypadat asi takhle:

<?php

namespace App\Front\Presenters;

use Latte;
use Project\Markdown;

class ArticlePresenter extends BasePresenter
{

    // ...

    protected function createTemplate()
    {
        $template = parent::createTemplate();
        $latte = $template->getLatte();
        $set = new Latte\Macros\MacroSet($latte->getCompiler());

        $set->addMacro('markdown', function (Latte\MacroNode $node, Latte\PhpWriter $writer) {
            return $writer->write('echo call_user_func($this->global->markdownToHtml, %node.args);');
        });

        $markdown = new Markdown();
        $latte->addProvider('markdownToHtml', [$markdown, 'transform']);

        return $template;
    }

}

Dalo by se s tím pracovat ještě více, ale toto jsem v dané situaci (když jsem tento kód řešil tady na blogu) považoval za rozumnou hranici.

Co mi tohle refaktorování přineslo:

  • Přehlednost - metody i třídy jsou stručné a řeší jednu věc.
  • Odstínění různých API od sebe (Latte, knihovna Michelf\MarkdownExtra, FSHL\Highlighter) - mohu je později vyměnit bez přepisování poloviny aplikace.
  • Přizpůsobitelnost - jestliže se má u nadpisů v článků generovat třeba anchor text, nebude těžké změnu zavést.
  • Testovatelnost - díky vlastnostem toho kódu můžu na Markdown napsat velice jednoduše testy.

Kdybych šel do extrémů, mohu ještě řešit:

  • Neregistrovat makro tímhle způsobem v presenteru do factory method, ale registrovat už někde v configu do nějaké abstract factory, řešit to třeba celé jinak.
  • Pro transformaci obsahu (článku) do HTML vytvořit rozhraní a to by pak implementovala třída pro transformaci Markdown i sanitizaci HTML co jsem řešil na začátku. Mohl bych pak snadno měnit, psát jiné.
  • Project\Markdown by si interně zachovávala jedinou instanci obalovaného Michelf\MarkdownExtra.
  • Project\Markdown by umožňovala konfigurovatelnost (např. nastavit prefix pro HTML ID).
  • ...

Shrnutí

  • Refaktorování je součástí naší práce (tzn. taky součástí odhadů; když se zde snaží ušetřit, naskáčou úroky v budoucnu).
  • Refaktoruji po malých krocích. Čím víc věcí najednou bych řešil, tím víc se v tom můžu utopit.
  • Buď refaktoruji nebo měním funkčnost. Ale nikdy obojí najednou.
  • Kód nebude nikdy dokonalý. Nesnažím se vyřešit všechny problémy lidstva. Nic se nemá přehánět, musí se to vyplatit.

Snažil jsem se vysvětlit princip, jak po sobě zanechat kód ve stavu, aby se s ním dalo později snadněji pracovat. Není to ani nic složitého a nemělo by to být ani nic drahého - naopak, mělo by se to vyplatit v dlouhodobějším časovém měřítku.

Refaktoroval jsem buď po dokončení nějaké implementace nebo před začátkem nové.

Nejhorší, co můžu udělat, je ponechat kód ve stavu, v jakém jsem jej na první pokus dovedl k funkčnosti.

Programování je lidská činnost. A je na mém uvážení, jak svědomitě kód napíši.

Kód bychom měli dodávat dostatečně kvalitní. A tak je po napsání vhodné jej projít a vylepšit, aby ta kritéria splňoval. Existují také techniky které pomáhají zvyšovat kvalitu při vývoji a už předem vedou k návrhu kdy kód bude napsaný hezky od začátku, např. z těch nejsnáze aplikovatelných Test-driven development a Code Review.

Literatura

Na závěr bych chtěl doporučit skvělou literaturu. Četl jsem je již před několika lety, ale získané know-how stále více a více využívám s tím, jak rostou mé dovednosti.

Pro člověka, co myslí programování vážně, by toto měla být povinná literatura.

Have you enjoyed this article?

Tweet