Mazaný a líný programátor

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

Vždycky jsem byl přesvědčen, že mazanost a lenost jsou dvě skvělé vlastnosti dobrého programátora. Vysvětlím to na kódu. Nejprve ukázka běžného přístupu:

<?php
final class Conventional {
    private $a;
    private $b;
    private $c;
    private $d;
    
    public function getA() {
        return $this->a;
    }
    
    public function setA(A $a) {
        $this->a = $a;
        return $this;
    }
    
    public function getB() {
        return $this->b;
    }
    
    public function setB(B $b) {
        $this->b = $b;
        return $this;
    }
    
    public function getC() {
        return $this->c;
    }
    
    public function setC(C $c) {
        $this->c = $c;
        return $this;
    }
    
    public function getD() {
        return $this->d;
    }
    
    public function setD(D $d) {
        $this->b = $d;
        return $this;
    }
}
?>

A teď řešení od mazaného a líného programátora. API není totožné, ale to je svým způsobem záměr:

<?php
final class SmartAndLazy {
    private $classes = array(
        'a' => 'A',
        'b' => 'B',
        'c' => 'C',
        'b' => 'D',
    );
    private $vars = array();
    
    public function __get($name) {
        return $this->vars[$name];
    }
    
    public function __set($name, $value) {
        if (method_exists($this, "set$name")) { // ošetření speciálních případů
            $this->{"set$name"}($value);
        } elseif (!array_key_exists($name, $this->classes)) {
            trigger_error("Property $name is not defined.");
        } elseif (!($value instanceof $this->classes[$name])) {
            trigger_error("Invalid class for property $name, must be " . $this->classes[$name] . ".", E_USER_ERROR);
        } else {
            $this->vars[$name] = $value;
        }
    }
}
?>

Druhé řešení považuji za v mnoha ohledech lepší. Jeho prvotní napsání bylo mentálně náročnější, při malém počtu vlastností asi i časově náročnější. Do budoucna nám ale přináší několik výhod:

Druhé řešení je podle mě krásná ukázka mazanosti (napíšeme kratší, i když složitější kód) a lenosti (přidání nové vlastnosti bude v budoucnu otázka jednoho řádku). Má i své nevýhody, i když čistě z technického pohledu mě napadá jediná:

Pokud bychom chtěli třídy doplnit o dokumentační komentáře, stálo by nás to u první varianty mnoho řádek balastního textu, u druhé varianty by to spláchnul jeden řádek @property pro každou vlastnost (ten je ale prakticky nezbytný, protože bez pohledu do třídy vůbec nemáme šanci zjistit, s čím pracovat lze a s čím ne).

Korporátní sféra

Ve Facebooku musí všechny změny schválit jiný vývojář. U prvního kódu je na první pohled jasné, co dělá, a bez problémů ho schválí prakticky kdokoliv. A to i přesto, že obsahuje snadno přehlédnutelnou a těžko odhalitelnou chybu.

Druhý kód naopak vyžaduje nemalé mentální úsilí nejen od toho, kdo kód schvaluje, ale i od všech dalších, kdo kód budou číst nebo s ním pracovat. Aby druhý kód vyhrál, musí mít opravdu zásadní přínosy a ušetřit do budoucna spoustu práce. Pár příkladů kódu od mazaného a líného vývojáře taky máme, ale ve většině případě je to spíš nudný kód podle prvního příkladu se všemi jeho nevýhodami.

Sám jsem se s tím také setkal: potřebovali jsme generovat dvě různé varianty celkem dlouhého HTML kódu, pro jehož vytvoření se používalo několik podmínek a cyklů. Vyřešil jsem to tak, že jsem vygeneroval kód jen jeden a pak ho v případě potřeby důmyslným způsobem převedl na druhou variantu. Na příkladu bych to mohl ukázat takhle:

<?php
$return = "";
foreach ($data as $key => $val) {
    if (is_valid($key)) {
        $return .= "<tr><th>" . htmlspecialchars($key) . "</th><td>" . htmlspecialchars($val) . "</td></tr>\n";
    }
}

if ($other_variant) {
    $return = preg_replace('~<tr><th>([^<]*)</th><td>([^<]*)</td></tr>~', '<span title="\1">\2</span>', $return);
}
?>

Změna mi sice byla schválena, ale ostatní vývojáři se jí báli, začali na ni svádět chyby, které vůbec nezpůsobovala, a nakonec ji smazali a nahradili jednoduchým kódem, který nejprve zjistil, která varianta se má generovat, a pak všechny cykly a podmínky provedl v obou větvích. Nějak takhle:

<?php
$return = "";
if ($other_variant) {
    foreach ($data as $key => $val) {
        if (is_valid($key)) {
            $return .= '<span title="' . htmlspecialchars($key) . '">' . htmlspecialchars($val) . "</span>\n";
        }
    }
} else {
    foreach ($data as $key => $val) {
        if (is_valid($key)) {
            $return .= "<tr><th>" . htmlspecialchars($key) . "</th><td>" . htmlspecialchars($val) . "</td></tr>\n";
        }
    }
}
?>

Jak tenhle kód vznikl? Copy/paste, samozřejmě. Co když budeme v budoucnu chtít udělat nějaké změny? Budeme je muset udělat na dvou místech, pochopitelně. Přesto se s tímto kódem většině lidí líp pracuje.

Co se náchylnosti k chybám týče, je to sporné. První kód je bez pochyby složitější, takže v něm lze snadněji něco pokazit. U druhého kódu je zase mnohem snadnější obě varianty rozhodit.

Co se dalšího rozvoje týče, je to taky sporné. Pokud budeme chtít obě varianty od sebe v budoucnu víc oddalovat, bude to jednodušší s druhou ukázkou. Pokud naopak budeme dělat stejné změny, tak u druhé ukázky uděláme spoustu zbytečné práce navíc.

Závěr

Často si proto při vývoji Facebooku říkám: „Nesnaž se být mazaný a nebuď líný.“ U svých projektů v mazaných řešení, které mi do budoucna ušetří práci, budu pokračovat, protože mi to činí potěšení. Ale ve firmě, kde se stejným kódem pracuje spousta lidí, jsou obvykle lepší jednoduchá řešení, která do budoucna práci neušetří, mají své problémy, ale každý je na první pohled pochopí. A taky je pravda, že chyby, které opravuji, mnohem častěji vznikly tak, že se někdo snažil být příliš mazaný, než že by zapomněl stejnou změnu na všech potřebných místech.

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

Diskuse

Michal Raška:

Jakube, mazaná řešení jsou fajn, ale nový programátor, který na nich bude chtít pokračovat, tě nejspíš bude chtít zabít obzvláště nehezkým způsobem. Protože, co ty jsi ušetřil, tak on ztratí dvojnásobně, než pochopí co to doopravdy dělá.

ikona Jakub Vrána OpenID:

Moje řeč. Mazaná řešení jsou dlouhodobá investice. A to ještě jen za předpokladu, že nebudou přicházet noví lidi. Což obvykle neplatí.

Nebo se taky hodí v situaci, kdy mi stačí „prostě to funguje“ a nepotřebuji v tom dělat změny. Např. v jQuery je pěkná řádka mazaných řešení, která ale nikoho netrápí, protože drtivá většina lidí s jQuery pracuje zvenku, málokdo se potřebuje hrabat vevnitř.

Michal Raška:

Co se týče různých knihoven, tak souhlas, ale taky to není bez rizika. Pokud to člověk používá tak, jak to je tak OK, ale ve chvíli, kdy ji potřebuješ přiohnout/doplnit/opravit může být lepší napsat si vlastní, protože to zabere méně času.

radix:

>> Moje řeč. Mazaná řešení jsou dlouhodobá investice. A to ještě jen za předpokladu, že nebudou přicházet noví lidi. Což obvykle neplatí.

To plati jen za predpokladu, ze kod je relativne maly. Ve vetsich projektech se casto stava, ze jeden programator upravi jednu tridu treba jen dvakrat za zivot (s nekolika letym odstupem) a pak tyto mazanosti fakt nepotesi.

Já:

Heleď a jak to máte ve Facebooku s použitím různých IDE? Asi bych už dneska nechtěl vedle sebe programátora, který nějaké nepoužívá, právě kvůli eliminaci těch "nejhloupějších" chyb...

ikona Jakub Vrána OpenID:

99% vi nebo emacs, já SciTE :-). I když osobně jsem IDE při práci na PHP nikdy nepoužíval, tak část firmy také považuje absenci podporovaného IDE za nedostatek. Ale žádné IDE by se bez úprav na našem kódu nejspíš nechytalo. Jen načíst ta kvanta souborů, které se navíc neustále mění, by asi většinu IDE položilo.

Michal Raška:

Hmm pro vás by asi bylo nejvhodnější IDE v clusteru ve spojení s tenkým klientem. Celá logika diff, vazby, kontroly verzování atd. by se řešily centrálně na serverech a do klienta by poslal jen to, co by bylo potřeba. Protože by se o to staral na jednom místě tak režie by byla nižší, než otvírat to decentralizovaně.

Otázka je zda to vůbec existuje. Pokud ne tak programátorů máte myslím dost ;)

ikona Jakub Vrána OpenID:

Otázka je, jestli by se do toho firmě vyplatilo investovat. Investovali jsme třeba do vlastního prohledávání kódu. `grep` byl moc pomalý, `git grep` byl moc pomalý, `scan` byl moc pomalý, takže máme vlastní distribuované prohledávání kódu, které si průběžně indexuje změny v jednotlivých projektech.

Michal Raška:

To je otázka. Záleží na tom, jak moc potřebu IDE ve firmě pociťujete. Ale jak se tak dívám, to prohledávání kódu je asi první krok.

Jinak vy asi z principu fungování FB nepoužíváte šablony, nebo se to podařilo obejít? Pokud nepoužíváte, jak udržujete generátory HTML?

ikona Jakub Vrána OpenID:

Prakticky celý kód je postaven nad XHP, takže žádné šablony neexistují. Pro mě osobně to byl docela nezvyk, prezentační logika je oddělena mnohem volněji, ale pracovat se s tím dá nakonec docela dobře.

Michal Raška:

Děkuji Jakube.

Honza:

Jenom bych chtěl upozornit na PHPStorm, jako naprosto výborné IDE na PHP, které jsem zatím potkal.
Je to hloupější než Visual Studio + ReSharper, ale jen jemně. A o mnoho chytrejsi a rychlejsi nez Netbeans.

MIchal:

Taky musim doporucit storm. Hlavne kvuli vykonu a minimalnimu obtezovani.
Prechazel jsem eclipse->netbeans->storm a pokazde slo o radovy narust rychlosti. Prijde mi ze storm je rychlej jak notepad, prestoze ma obrovsky mnozstvi pomocnych funkci. Co udela na obrovskym projektu s rozsahlym stromem souboru ale netusim.
Urcite zkuste aspon trial vsichni kdo jste ho nevideli.

Filip Procházka (@HosipLan):

Mám v projektu stovky tříd a úplně bez problému. Ale když zkouší našeptávat něco, co připomíná funkci z minifikovaného admineru, tak vymrzne na půl minuty :D Řešením je dát na adminer exclude, aby ho neindexoval.

MIchal:

Jo s tim jsem se taky setkal. Velke fily to spatne dava. Mam jeden projekt na stare symfony 1.4 a v nem asi tak 2000 radkove integracni testy - neda se :-) . testuju jejich casti v temp test filech a nasledne je presouvam do celku. Mam to v tom nahromade kvuli prehlednejsim vyjezdum pri uzivani.

ikona Michal Špaček:

Pod závěr bych se podepsal. Ve Skype jsem se naučil psát kód "pro kolegy", ne pro sebe. Protože ve většině případů jsem kód napsal a za nedlouho na něm začal pracovat někdo jiný. A není větší zábavy, než když je třeba něco opravit jakože hned a autor mazaného kódu má dovolenou ;)

Mimochodem, pokaždé, když vidím HTML a regulární výrazy dohromady, tak se mi chce dumpnout core (a možná i to způsobilo strach ostatních?), ale jako příklad to chápu.

kevujin:

Není se čemu divit, když máš uchylku na hardcore c like shell brutal výrazy. Aneb ušetři bajt, zničíš tím život.

ikona Jakub Vrána OpenID:

Taky jsem váhal, jestli mám v článku regulární výraz použít. Skutečný kód používal samozřejmě XHP a značky konvertoval pomocí jeho API, nikoliv regulárním výrazem. Ale ukázku by to zbytečně zkomplikovalo, tak jsem zvolil jednodušší (i když hackerštější) řešení.

ikona David Grudl:

Z regulárních výrazů už mám pomalu strach, kdyby nějaký prvek v $data byl moc velký, PHP zachová poker face a v naprosté tichosti vrátí nesmysl.

Richard Šerý:

Při vývoji sice obvykle propaguji taková řešení, aby to pochopilo i malé dítě (a tedy i já), občas však takové řešení vede k velké redundanci a nakonec i k chybám. Pak bych souhlasil s kódem, který je "mazaný", ale musí dodržet dvě podmínky:
1) Objekt, metoda či funkce, obsahující "mazaný" kód, musí být velmi chytře nazvaná, aby nikdo nebyl na pochybách, co to dělá a proč.
2) Musí tam být komentář, vysvětlující, že kód je záměrně méně čitelný a proč tomu tak je. Když narazím na pasáž, které nerozumím, mám tendenci se v tom vrtat. Když je tam ale jasně napsáno "magie která dělá to a to, nepokoušejte se to pochopit", tak mi stačí, že jedou testy a dál to obvykle neřeším.

Substance242:

Už veľmi neprogramujem, ale to prvé nie je príklad programovania, ale odfláknutého hnusu, ble. Nepovažujem za správne prispôsobovať sa tým hlúpejším.

MIchal:

Notak hlavne ze sis ulevil '-)

Richard:

Princip DRY je pravidlo jako každé jiné - vyplatí se ho dodržovat a ve výjimečných případech se ho vyplatí porušit. Pokud nepovažujete za správné přizpůsobovat se těm hloupějším, pak je myslím dobře, že moc neprogramujete.

v6ak:

První příklad (get/set): To dost závisí na podpoře v jazyce. Právě kvůli slabé podpoře (interfaces) to v PHP nepoužívám. Ono je pak trochu matoucí programovat jinak, pokud mám implementovat interface. Rád ale použiju něco jako: case class (a: A, b: B, c: C, d: D); // podpora v jazyce Scala je slušná a vestavěná

Druhý příklad: Obě varianty se mi nelíbí. První je, no, obskurní, a druhá popírá DRY. Ideálně bych to napsal ve stylu:

val mappingFunction: (Entry[KeyType, ValueType] => String) = if(otherVariant){
    e => <span title={e.key}>{e.value}</span> toString;
}else{
    e => <tr><th>{e.key}</th><td>{e.value}</td></tr> toString;
}
data.filter(_.key.isValid).map(mappingFunction).join("\n")

To mi (zhruba) dovolí Scala. (Kód není 100% Scala. Striktně vzato, je to pseudokód velmi podobný Scale a měl by být pro neznalé o něco čitelnější.) V PHP to sice jde, ale s array_map a array_filter bude méně přehledné. Navíc je tu horší podpora lambda funkcí.

V PHP se nabízí ještě tato varianta, která mi přijde lepší než ta první:
<?php
if ($other_variant) {
    $before = '<span title="';
    $between = '">'A;
    $after = "</span>";
} else {
    $before = "<tr><th>";
    $between = "</th><td>";
    $after = "</td></tr>\n";
}
foreach (
$data as $key => $val) {
    if (is_valid($key)) {
        $return .= $before . htmlspecialchars($key) . $between . htmlspecialchars($val) . $after. "\n";
    }
}
?>
Má tato varianta oproti té úsporné s regexpem nějakou nevýhodu? Výhody vidím ve srozumitelnosti, rychlosti i snadnosti úprav.

Variantě s regexpem bych napak vytknul, že pokud upravím tu tabulkovou variantu, musím upravit i regexp - to je IMHO horší duplicita než u těch smyček s ifem.

ikona Jakub Vrána OpenID:

Výborný postřeh! Tohle je smysluplné třetí možné řešení.

Proč jsem původně zvolil první? Kvůli lenosti. Nechtělo se mi v kódu přepisovat všechny výskyty "<tr><th>..." na volání funkce, která by se rozhodla, co vrátí.

Proč nakonec vyhrálo druhé? Změny ve větvích se začaly čím dál víc rozcházet, už nešlo jen o změnu značek, ale i o to, že se některé části přesunuly na jiné místo, případně zůstaly jen v jedné větvi.

Ale tebou popsané kompromisní řešení by asi v řadě případů bylo nejlepší.

ptica:

ukol bych resil pomoci jednoho controlleru a dvou template,
lepeni stringu v php u me vzdycky skonci totalni ztratou orientace v uvozovkach vsech typu

lopata:

přesně tak, jeden cyklus a v něm if ($other_variant) je jasně nejlepším kompromisem.

Martin Soušek:

Koukám že byste si měli přečíst knihu: "Clean Code: A Handbook of Agile Software Craftsmanship". Tam to všechno je.

A jako domácí úkol si zkusit refaktorovat první i druhou variantu. Protože s prominutím - první příklad s regulárním výrazem i druhý příklad s copy/paste je pořád ultimativní blbost.

ikona Jakub Vrána OpenID:

Hlavně, že máte ve všem jasno. Viz také http://php.vrana.cz/mazany-a-liny-programator.php#d-13223 a http://php.vrana.cz/mazany-a-liny-programator.php#d-13226

Martin Soušek:

Nesouhlasím.

radix:

Nesouhlasim.

Pavel:

Souhlasím s vámi.

První řešení je čisté a argumenty proti, které jsou uvedeny v článku, jsou nesmyslné. Prostě proto, že přidání nové vlastnosti je záležitost definování property a vygenerování setterů a getterů od IDE, žádná chyba nehrozí.
Naopak za to dostane funkční našeptávač, zdokumentované metody a standardní rozhraní, na které jsou všichni uvyklí.

Druhé řešení je cesta do pekla a nejpozději při příští změně kódu na něj někdo doplatí. Minimálně zaměstnavatel programátora, který bude muset vyvinout "nemalé mentální úsilí", aby vůbec pochopil, co má pro přidání té jedné nové vlastnosti udělat. A ten programátor může být i původní autor, protože po dvou měsících už bude tušit stejný prd jako kdokoliv jiný.

Pavel:

aha, vy řešíte tu druhou sadu příkladů, já reagoval na "Conventional vs SmartAndLazy"

ikona David Grudl:

Je dobrým zvykem tento typ komentářů začínat slovy „Autor by si měl …“, aby je čtenář mohl snáze filtrovat.

Olda:

Obojí varianty jsou možné, rozhodoval bych se spíše k mazané verzi).
Samotného mne napadlo oddělit html značky do proměnných (jako to udělal v6ak) pokud by to ovšem nevedlo ke zesložitění kódu.

ikona Knyttl:

<?php
$return
= "";
$format = $other_variant
     
? '<span title="%s">%s</span>'
      : '<tr><th>%s</th><td><tr>';

foreach (
$data as $key => $val) {
      if (is_valid($key)) {
            $return .= sprintf($format, htmlspecialchars($key), htmlspecialchars($val);
        }
    }
}
?>

Tohle mně osobně přijde nejčitelnější pro okolní svět a zároveň nejjednodušší.

ikona Jakub Vrána OpenID:

Viz http://php.vrana.cz/mazany-a-liny-programator.php#d-13226.

ikona Knyttl:

<?php : '<tr><th>%s</th><td>%s</td><tr>'; ?>

Tady mi jen vypadlo to druhé <td>.

Sniper:

No nevim, ale k tomu prvnimu prikladu - nadefinuju si ty properties a reknu netbeansum "vygeneruj mi gettery a settery" a hotovo. cili za me je u prvniho prikladu resenim cislo jedna lepsi - kdokoliv ho hned pochopi, je jasne a diky IDE jeho vytvoreni trva kratsi dobu s defakto nulovou sanci na preklep

ikona Knyttl:

To v Javě jde pomocí anotací udělat něco jako:

<?php
class Conventional {
    @Setter @Getter
   
private $a;

    @Setter @Getter
   
private $b;

    @Setter @Getter
   
private $c;

    @Setter @Getter
   
private $d;
?>

- což mi přijde výborně čitelné a udržitelné.

radix:

No v Jave SE/EE rozhodne ne.

Taktez je docela problem, ze toto IDE nepobere (pokud neobsahuje podporu pro tyto konkretni anotace).

ikona Knyttl:

Je na to nástroj lombok a plugin je dostupný ke všem IDE.

radix:

"vsechny IDE" jsou 2, jo? :-) Pro me je docela dulezite, ze IntelliJ IDEA podporovana neni.

Jinak narazel jsem hlavne na to, ze jsi napsal "v Jave". Je velky rozdil mezi "Java" a "knihovna pro Javu od treti strany". Kazda zavislost na externi knihovne musi byt dobre opodstatnena, uz vidim, jak za 5 let budu prechazet na novou Javu, kterou Lombok nepodporuje (protoze uz je 2 roky mrtvy) a ja budu muset prepisovat pulku kodu.

ikona Knyttl:

To teda podporována je, jen to nemají na ofiku napsáno :-) Já to používám právě v Idee :-)

Určitě se nechci dohadovat, ale lombok tu mojí lenost řeší výborně. Jen je škoda, že ty vygenerované Settery nejsou fluent. Umí to další supr věci, třeba @Slf4j. Na to nedám dopustit.

ikona Jakub Vrána OpenID:

Jak by kód vypadal, když bych u některých vlastností vyžadoval, aby musely být před získáním nastavené, a u jiných ne?

Franta:

Potřebuješ rozlišovat mezi a) nebylo nastaveno a b) bylo nastaveno null?

ikona Jakub Vrána OpenID:

setA(null) stejně neprojde, takže odpověď je nejspíš ne.

Franta:

Proč by nešlo (nebo nemělo jít) setA(null)?

Snažím se přijít, proč to vlastně potřebuješ, resp. proč ten předchozí dotaz – jestli to má nějaký praktický význam, nebo jen chceš najít něco, co Lombok neumí…

A jinak vtip anotací v Javě je, že si můžeš vytvořit vlastní – stejně jako někdo vytvořil Lombok – takže to může dělat přesně to, co chceš (třeba vyhazovat výjimku, když nebyl příslušný setter zavolán).

ikona Jakub Vrána OpenID:

Promiň, nějak jsem si nevšiml, že jsme ve vlákně, které se baví o Javě. V PHP jde NULL předat jenom v nepovinných parametrech. Proto jsem psal, že setA(null) vůbec nejde.

A ano, anotace (ne, nekoktám) mají své kouzlo, obecně mít možnost jít nad možnosti daného jazyka. V Nette podobnou volnost poskytuje Latte, viz např. http://php.vrana.cz/notorm-2.php#latte.

Jakub Tesárek:

Určitě tu padne hodně příkladů o tom, proč je to špatně. Já ale přidám jeden a asi ten nejdůležitější, proč nelze v tom prvním příkladu použít ten druhý přístup. Nebude to totiž fungovat. A to nemluvím o nějakém překlepu, kterého jsem si popravdě nevšiml. Je tam velmi dobře schovaný a přitom zásadní bug. Modří už vědí.

A samozřejmě nesmím zapomenout zmínit, že používat reflexi na nasetování property je omluvitelné snad jen u Doctrine a Unitestů.

ikona David Grudl OpenID:

U Doctrine to omluvitelné není ;-)

ikona Jakub Vrána OpenID:

Nechám se poddat.

Jakub Tesárek:

... spíše jsem měl použít "jen kvůli nasetování" než "na nasetování"

ikona David Grudl OpenID:

Rozdíl mezi konvenčním a mazaným řešením bude v tom, že konvence jsou od toho, aby pomáhaly lidem se navzájem dorozumět (a to klidně i sebe se sebou, při pohledu na vlastní starý kód), zatímco mazanost šikovně řeší nějaký problém. Soft skills versus programming skills.

Tedy i řešení ala třída SmartAndLazy může plnit první roli, pokud se v rámci týmu stane konvencí.

Nicméně zrovna tahle konkrétní třída SmartAndLazy mi přijde z mnoha úhlů jako dost špatné řešení, už proto, že přidání nové vlastnosti _není_ otázka jednoho řádku, ale neřešitelný úkol (vytvoř class MissionImpossible extends SmartAndLazy s vlastností $e třídy E).

ToM:

$classes zmenime na protected a vlastnost pridame v konstruktoru potomka, oj to to boli, to uz neni vubec pekne.

ikona David Grudl OpenID:

…a nesmíme zapomenout v dalších potomcích volat parent::__construct(), jinak vzniknou špatně odhalitelné chyby. Navíc zcela zmizí jakákoliv statická definice třídy. Takže ano, to není vůbec pěkné.

ikona Jakub Vrána OpenID:

Máš pravdu, že tohle je zcela zásadní problém, který by znamenal nutnost kompletního přepsání, pokud by to někdo chtěl opravdu použít. Stejně jako na první variantě druhého příkladu je na tom vidět, že jsem to nasmolil jen pro účely článku.

Nicméně jsem to „vyřešil“ :-).

dynínská výzva:

Víš, že d,b jsou jiná písmena? :)

David Grudl:

Víš, že je lepší nejprve článek přečíst a pak komentovat, jinak ze sebe uděláš troubu?

ikona Ondřej Mirtes:

Nikde tu nezaznělo ani slovo o testech. TDD přístup navrhuje přistupovat k vývoji tak, že se nejdřív napíše failující test, požadovaná funkcionalita se co nejjednodušeji/nejrychleji (klidně copy-paste) naimplementuje, aby test prošel (zezelenal) a pak se refaktoruje implementace tak, aby test zůstal zelený a programátor byl s podobou kódu spokojený. Při tomto postupu se pak můžou implementace prostřídat a modifikovat, aniž by se kdokoli bál, že vytvoří něco zpětně nekompatibilního.

Nebál bych se tedy psát mazaná řešení, pokud pomohou odstranit duplikovaný kód nebo vyprodukovat kompaktnější, ale samozřejmě ne za cenu přehlednosti. Pokud je otestovaný, nikdo se nemusí bát na něj sáhnout.

mj41:

Jakube těším se na další články na téma, co jsem se ve FB naučil dělat jinak.

Bude například zajímavé, jak to ovlivní další vydání tvé knihy :-).

Já osobně, jsem na tvých školeních třeba nesouhlasil s tím, že doporučuješ ignorovat E_NOTICE. Mě se každé drbání s warningy u trochu složitějších aplikací (např. registrace předmětů na VŠ), vždy alespoň desetínásobně vrátilo. Zajímalo by mě jak to máte s E_NOTICE ve FB? Díky.

ikona Jakub Vrána OpenID:

Já bych E_NOTICE milerád používal, když by fungovaly smysluplně. Už jsem to několikrát popisoval, možná to je i v knize, určitě to je na školení Bezpečnosti. Co mi na E_NOTICE překáží:

<?php
$counts
= array();
foreach (array(
/* ... */) as $row) {
    $counts[$row["group"]]++; // E_NOTICE k ničemu
}
?>

Co mi naopak chybí:
<?php
$a
[0] = 1; // žádná E_NOTICE, přesto může jít o vážnou chybu
?>

Další problém je v tom, že se E_NOTICE vygenerují až při spuštění, takže pokud je problém v nějaké zastrčené větvi, tak už na opravu může být pozdě.

Proto raději vsázím na statickou analýzu: sám jsem si napsal http://code.google.com/p/php-initialized/, které mi už mnohokrát pomohlo a které funguje v podstatě podle mých představ. Pomocí něj jsem kdysi našel i dvě těžko odhalitelné chyby v Nette, které používání E_NOTICE podporuje a je psané velmi pečlivě. Jiné projekty jsem raději nekontroloval…

Statická analýza pomůže odhalit i obraty, které jsou z pohledu E_NOTICE v pořádku a které mohou být i v platném programu, obvykle jsou to ale chyby. Třeba:

<?php
foreach (array(1) as $row) {
}
var_dump($row); // E_NOTICE mlčí, php-initialized zařve
?>

Ve Facebooku to máme podobné. Statickou analýzu používáme v mnohem větším rozsahu (např. https://github.com/facebook/pfff). Odhalíme třeba i takovéto chyby:

<?php
$row
= 1;
foreach (array(
1) as $row) {
}
var_dump($row); // E_NOTICE mlčí, php-initialized mlčí, ArcanistXHPASTLinter zařve
?>

A z chyb nás zajímají hlavně ty fatální. A to ještě jen pokud jich je víc než milión denně ;-).

Takže v tomhle se můj postoj prakticky nehnul.

ikona Jakub Vrána OpenID:

A ještě jedna věc mi na E_NOTICE vadí:
<?php
if (isset($my_long_variable[$my_long_index])) {
    $x = $my_long_variable[$my_long_index];
    // ...
}
?>

Bez E_NOTICE je to mnohem hezčí:
<?php
$x
= $my_long_variable[$my_long_index];
if (isset(
$x)) {
    // ...
}
?>

ikona Jakub Vrána OpenID:

Ještě k tomu „doporučuješ ignorovat E_NOTICE“:

Na to jsem si vždycky dával pozor a nikdy jsem to nedoporučoval. Vždy jsem se k tomu otevřeně hlásil, vždy jsem uváděl důvody, které mě k tomu vedou, ale zároveň jsem říkal: „Pro vás může být E_NOTICE užitečné, i když pro mě není. Obzvlášť, jestli máte v týmu nováčky, nebo nepoužíváte statickou analýzu.“

mj41:

Díky, ta statická analýza dává smysl i když v mém případě (jak si matně vzpomínám) byl kód hodně řízen daty, tak by asi pokaždé nezabrala. Každopádně díky za tip, i když už asi nebudu mít příležitost jej vyzkoušet, protože jsem zpět u Perlu (btw [1]).

Už je to dávno, takže si to ze školení asi moc přesně nepamatuju. Z mých zkušeností se E_NOTICE vyplatí u všech složitějších algoritmů. To jestli je píše nováček nebo profesionál je docela jedno.

[1] http://mj41.cz/wiki/GoodData_Perl_career

lopata:

perl 6 rulez!!!

ikona David Grudl OpenID:

Co je tady za chybu?

<?php
$row
= 1;
foreach (array(
1) as $row) {
}
var_dump($row); // E_NOTICE
?>

ikona Jakub Vrána OpenID:

Jak jsem psal, jde o ukázku platného programu, který ve výjimečném případě může dávat i smysl. Ale většinou si tímhle kódem přepíšeš nějakou proměnnou, kterou si přepsat nechceš. Zkusím to ukázat na lepším případu:
<?php
class C {
    function processControl($control) {
        foreach ($this->childControls as $control) {
            // ...
        }
        // Co teď bude $control? Záleží na okolnostech.
        $control->setProcessed(true);
    }
}
?>

Vlastně to je stejně hloupá chyba jako:
<?php
for ($i=0; $i < 10; $i++) {
  for ($i=0; $i < 20; $i++) {
  }
}
?>
Taky jde o platný program, který ve výjimečném případě může dávat smysl a na který E_NOTICE neupozorní.

ikona David Grudl OpenID:

Aha, špatně jsem to pochopil.

David KONEČNÝ:

Pokud má "mazané" řešení patřičný komentář, který umožňuje kód pochopit i méně zdatným programátorům, pak je dle mého názoru lepší volbou.

ikona David Majda OpenID:

Souhlasím s tvým závěrem nebýt mazaný při vývoji ve firmě. Nicméně já tuto filozofii většinou uplatňuju i v čistě svých projektech, a to ze dvou důvodu: Skoro nikdy nevím, jestli projekt bude navždy můj, nebo do něj časem bude sahat i někdo jiný (protože ho opensourcuju, někdo ho po mě převezme, apod.). Krom toho předpokládám, že v daném jazyce/prostředí nebudu dělat navždy a moje znalosti pokročilejších věcí se časem vytratí.

To druhé se mi teď stalo s PHP -- dva roky v něm aktivně nedělám, dost věcí jsem zapomněl, ale stále udržuju některé starší projekty a občas do nich musím zasáhnout. Za každý řádek kódu, který jsem nenapsal mazaně, jsem teď rád (a naopak).

K tvému řešení s regulárními výrazy: Podobně jako jiným se mi nelíbí ještě víc než řešení s duplicitou. Dá se na něm ale alespoň krásně ukázat, jak se dá takové špatné řešení poznat z jeho strukturálních vlastností, aniž by bylo třeba důkladně zkoumat kód:

1. Dvě symetrické situace se v něm řeší asymetricky. To znamená, že kód svou strukturou nemodeluje zadání, což je často signál o špatném návrhu. Asymetrie pak taky skoro jistě znamená, že někde je použita nějaká zkratka. Zkratka je v podstatě synonymum pro hack.

2. Kód zavádí závislost mezi jednotlivými případy. Pokud se bude měnit generovaný kód v prvním případě, budeš nejspíš muset měnit i regulární výraz pro případ druhý. Lze si snadno představit, že z toho budou plynout chyby a zvýšené náklady na údržbu, které převýší podobné náklady na údržbu prosté duplicity.

Osobně bych vše vyřešil tak, jak navrhuje někde výše v6ak. A nebo bych to neřešil vůbec -- ve chvíli, kdy je duplicitní kód jen na dvou místech a hned u sebe, je dle mé zkušenosti důsledné dodržování DRY spíš práce navíc. Krom toho se dost často stane to, cos popisoval v komentáři výše -- kopie se *záměrně* rozejdou. Obvykle tak refaktorizuju až při třetí kopii nebo když se obě kopie od sebe mají vzdálit.

ikona David Majda OpenID:

Ještě bych dodal k bodu 2., že závislost mezi případy zavádí až kód, v zadání nebyla, čili opět mají zadání a řešení jinou strukturu. V původním komentáři ten bod vypadá spíš jako námitka pragmatika, protože rozebírám důsledky, ale chtěl jsem hlavně upozornit na tu (ne)podobnost struktury.

lopata:

Trochu je vidět, že si ve FB skončil asi někde ve frontend týmu, takže v podstatě u šablono špaget?

ikona Patrik Šíma:

To mi silně připomíná diskuze okolo vývoje linuxového jádra :)

František Musil:

Napadlo me, jestli se nahodou do vyvoje PHP ve FB nevmesuje ten fakt, ze je podstatna cast kompilovana HipHopem. Tim padem si programator muze dovolit psat dlouhy a jednoduchy kod, protoze stejne to tak jako tak kompilator (resp optimalizator v c++) uplne prekope. Pak bych rekl ze neni tak dulezita efektivita a rychlost, jako prehlednost. Casto se s tim setkavam v nizsich jazycich, ze je lepsi psat kod zdlouhaveji a srozumitelneji, protoze vysledek po kompilaci je stejny.
S tim uzce souvisi i otazka, jak moc je pak vyvoj PHP timto faktem ovlivnen?

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.