U toho ifu je důležité, zda obsahuje business logiku. A zrovna tenhle if ji obsahuje, protože definuje, jaké username se použije, pokud uživatel není přihlášen. Pokud proměnná user je statická a její hodnota se naplní jen jednou, pak není co řešit a zůstává to jako if. Pokud je to ale operace, která se vyskytuje v kódu vícekrát, pak jednoznačně patří do samostatné funkce. Účel poznáte ve chvíli, kdy budete chtít logiku změnit, kupříkladu místo 'guest' použít 'anonymouse'.
Zjednodušeně řečeno je moje doporučení: pokud ta část kódu dělá nějaké rozhodnutí, které se dělá shodně na více místech, tak ji má smysl vyčlenit jako samostatnou funkci tak, aby ve zdrojovém kódu byla ta část kódu zapsána jen jednou. A to i když se jedná o jediný řádek.
V podstatě souhlas, ovšem s jednou otázkou: Jak v rozsáhlém projektu poznám, že se nějaké rozhodnutí dělá na více místech? V tuto chvíli se dělá na jednom, ale co zítra? Mám při refaktorování pátrat, na kolika dalších místech se ta podmínka vyskytuje?
Vyhledávání, zejména ve velkém projektu, je největší brzdou. Proto je nutné vyčlenit business logiku do třídy a metody, která za ni bude zodpovědná. Tam se nám najednou tyto metody začnou scházet, tam kolega při pokusu o refaktorování jiného bloku narazí na to, že jméno metody je již obsazeno a že ta metoda dělá přesně to co chtěl. Pokud ne, tak ji pojmenuje jinak.
Proto tvrdím, že další výskyty je zbytečné hledat, protože bychom přitom vypadli z kontextu právě refaktorovaného kódu a vyplýtvali bychom zbytečně mnoho času opětovným navazováním. Je nutné se soustředit výhradně na blok, který právě refaktoruji.
Je také nutné se každého kandidáta na vyčlenění ptát: Kam patříš? Ti, kteří si nedokáží odpovědět na takovou jednoduchou otázku, strkají takové bloky do nejrůznějších registrů a tříd obsahujících jen statické metody. Typickým registrem je config, typickou statickou třídou je třeba Math. Netvrdím, že jsou špatné a zbytečné, ale je nutné se ubránit choutkám k nim přidávat cokoli dalšího, co může být delegováno do třídy, do které patří.
Krásnou ukázkou chybné implementace v OpenCart, je například databáze. Hned v index.php je kód:
$db = new DB(DB_DRIVER, DB_HOSTNAME, DB_USERNAME, DB_PASSWORD, DB_DATABASE);Je zde použito 5 konstant, z nichž ani jedna není na předchozích řádcích definována. Je mi jasné, že je to v některém z předchozích require_once(), ale proč bych je měl prohledávat? Ve svých aplikacích to řeším takto:
$db = new DB($config);Třída DB dostane v parametru možnost nahlédnout do $config a vytáhnout si potřebné údaje. Nikde žádná konstanta, kterou bychom museli vláčet celou aplikací. Pokud chci použít jinou DB, dám jí jiný $config. Už se však nemusím přetahovat o globální konstanty.
Proč se ta DB ihned otevírá, když ještě ani nevíme, zda ji budeme potřebovat? Přitom stačí ve třídě DB udělat odloženou inicializaci, která nám zajistí podobný efekt jako populární Singleton a přitom nás neomezuje v počtu současně otevřených databází.
Příkazy require_once ani include_once zásadně nepoužívám. Od doby, kdy funguje autoload, jsou nepotřebné. Obvykle si v jedné aplikaci vystačím s jediným require. Výjimkou jsou některé cizí knihovny, které jsou tak dobře napsány, že je zbytečné je vynalézat znovu.
16 Odpovědí
3974 Zhlédnutí