Mirin webspace

Nejbohatší život má ten, kdo žije s minimem nároků

18. 7. 2012 - Komentáře (8) PHP Nette

Hovno kód v Nette?

Když jsme se nedávno s kolegy konečně dokopali k upgradu Nette, tak na světlo vyplynula jedna trochu nepříjemná skutečnost a to v základu celého frameworku - v komponentovém modelu.

Lecos už naznačila ukázka jednoho z pilířů celého frameworku - třídy Nette\Application\UI\Presenter, kterou si dokonce dovolil nějaký neznaboh označit za "hovno kód".

class Presenter
{
   ....
   ....
 
    /**
    * Returns self.
    * @return Presenter
    */
    final public function getPresenter($need = TRUE)
    {
        return $this;
    }
}

Následně byl hned umravněn komentářem ve smyslu "tohle nesmrdí, vždyť přece Presenter dědí od Control, takže je to v pořádku". Opravdu je to v pořádku?

Když se podívá na ukázku kdokoli bez nějakých znalostí frameworku, tak je to minimálně podivné. Volat takovou metodu postrádá přeci smysl, navíc ten nesmyslný parametr. Ať je parametr $need naprosto cokoli, dostanu pokaždé to samé.

Presenter není komponenta

Třída Presenter opravdu musí v současnosti implementovat metodu getPresenter, protože dědí od třídy Control. Najdete to dokonce i v dokumentaci o presenterech. Ale je to skutečně správně?

Třída Control představuje to, pro co se v dokumentaci a všude v diskusích okolo Nette používá slovo "komponenta", ve zkratce je to znovupoužitelná část zobrazené stránky. Právě tuto zobrazenou stránku představuje ve frameworku třída Presenter. Je tedy Presenter - zobrazená stránka - opravdu speciálním případem komponenty - své části - jak je to v současnosti? Presenter podle všeho potřebuje disponovat některými vlastnostmi komponent

  • obsahovat různé množství jiných komponent, které mohou obsahovat další komponenty - stromová struktura a operace nad ní
  • být vizuální - umět pracovat se šablonami a vykreslit se
  • obsluhovat volání akcí představovaných signály (ale šlo by to i bez nich, viz. dále)

Naopak nepotřebuje, nebo by dokonce neměl mít, tyto vlastnosti komponent

  • být obsahem jiné komponenty - představuje stránku, tedy nejvyšší kořen stromu komponent
  • dohledávat nadřazený presenter (obecně svůj kořen) - pokud hledám sám sebe tak se mnou není něco v pořádku

Hlavním úkolem presenteru také není být skladem na komponenty (potažmo komponentou), jeho hlavní odpovědnost je odbavit daný požadavek. K té zmínce o signálech - presenter by obsluhovat signály nemusel, není k tomu důvod, tohle se může klidně přenechat pouze komponentám. Na druhé straně signály v preseterech zjednodušují odbavení některých - zejména nevizuálních - akcí.

Z výše uvedeného plyne, že Presenter nemá být potomkem komponenty, není jejím speciálním případem a výše uvedené společné vlastnosti má s komponentami sdílet jiným způsobem než prostřednictvím dědičnosti.

Špatná viditelnost metod

Dalším - možná trochu neočekávaným - důsledkem nevhodnosti dědičnosti presenterů od komponent je použití protected metod presenteru ve třídě Control. Takto to vypadá v metodě Control::createTemplate()

protected function createTemplate($class = NULL)
{
  $template = ...;
  $presenter = $this->getPresenter(FALSE);
 
  if ($presenter instanceof Presenter) {
    ...
    $template->netteHttpResponse = $presenter->getHttpResponse();
  }
 
  ...
}

Problematická je metoda Presenter::getHttpResponse(). Tato metoda je totiž ve třídě Presenter - potomkovi Control - definována jako protected. Ve třídě Control je možné ji zavolat, protože PHP samotné umožňuje volat protected metody potomka v předkovi.

Problém je v tom, že ve svých aplikacích dědíte své komponenty právě od třídy Control. V těchto vašich komponentách však už Presenter::getHttpResponse() nezavoláte, protože Presenter není potomkem vašich komponent.

Těžko říci, kolik toho v tomto případě jde na vrub výše zmíněné vlastnosti PHP. Jisté ale je, že pokud by Presenter nebyl potomkem Control, jeho metody volané ze třídy Control by nikdy jako protected nevznikly, musely by být public. Jako řešení je proto nutné protected metody třídy Presenter používané ve svých komponentách učinit ve svých presenterech jako public, případně si udělat drobný patch do frameworku, který se dá přežít.

Komponentový model popsaný výše je v Nette od jeho počátku a jeho změna je radikálním krokem, který znamená přepsat základy frameworku. Uvidíme, zda se tak někdy stane. Je pravda, že nějaké zásadní obtíže, které by se nedaly řešit, současné uspořádání nepřináší, takže se nemyslím, že k tomu v dohledné budoucnosti dojde. Na konci zmíněný problém s viditelností některých protected metod presenteru v komponentách je jednoduše řešitelný (změnou na public) i bez zásahu do komponentového modelu.

Abych jen nevytahoval nedostatky, je nutné zmínit, že ve verzi 2 Nette ušlo oproti předchozí verzi 0.9 pořádný kus dopředu zejména v oblasti DI a dokumentace se také podstatně vylepšila, upgrade se určitě vyplatí.


Komentáře (8)

  1. Tharos - 20. 7. 2012 01:25

    Aneb kdo chce psat bít. :) Jde o známý problém a již před dlouhou dobou se o nevhodnosti aktuálního řešení opakovaně rozpovídal i David Grudl. Takže mi to celé připadá jako takové trochu (zbytečné) ?kopání do mrtvol?. Tedy kromě toho s return $this, které je vážně perla. :)

    Toto je, alespoň co já vím, velmi stará část frameworku. V podstatě to takhle bylo navrženo Davidem snad někdy v roce 2003. To znamená, že ten návrh je devět let starý. Ukaž mi nějaký Tvůj devět let starý kód a já si taky kopnu ;). Myslím to s nadsázkou, ale popravdě když si vezmu nějaký svůj kód z té doby, ?hovno kód? je slabé slovo.

  2. Tharos - 20. 7. 2012 01:28

    A oprav ten hovno kód, který na českém blogu převádí české uvozovky (??) v komentářích na otazníky ;).

  3. koubel - 20. 7. 2012 08:32

    [2] - Výborně, pokud se o tom i někde rozepsal, tak bych si to rád přečetl. I podle toho komentáře na webu s kódem o tom asi ví málokdo.

    Jako kopání do mrtvoly nebo bití psa bych článek nenazval, titulek je bulvární ale obsah už ne.

    Já napíšu občas hovno kód i dnes, na tom nic není, to se prostě stává.

  4. Tharos - 20. 7. 2012 10:09

    Na fóru je o tomto zmínka například zde. Dále má tento problém bod v roadmapě (která již ale dlouho nebyla aktualizovaná, takže kdo ví, jak to s ní je).

    O dekomponování Presenteru se (i včetně zachování zpětné kompatibility v rámci možností) pokoušel/pokouší Vašek Purchart v rámci nějaké školní práce na ČVUT, ale netuším, jak daleko se s tím dostal. I udržení zpětné kompatibility by mohlo být do jisté míry možné, protože presenter může do značné míry vystupovat jako fasáda a jen pod pokličkou delegovat to, co teď dělá sám, na subsystémy. Pak to bude i návrhově čisté.

    Označení článku tedy beru zpět. Já jsem na bulvární titulky trochu háklivý. Ihned očekávám od autora předpojatost a asi jsem pak předpojatý i já sám. :)

  5. Filip Procházka (@HosipLan) - 21. 7. 2012 13:41

    Díky Tharos, že jsi kus napsal za mně :)

    Co se týče metody getPresenter(), tak té imho vůbec nic není. Je to pouze optimalizovaná metoda rodiče. Důsledek toho, že dědí do Control a PresenterComponent - polymorfismus.

    To, že by Presenter nemusel dědit od Control, Container z component modelu by tam měl být přes traitu a vůbec by měl presenter vypadat úplně jinak je podstatnější problém, kterým se zabýváme už delší dobu. Tedy energii bych směřoval spíše tímhle směrem.

    Imho celý tenhle problém v podstatě vznikl, protože když to David navrhoval, neměl k dispozici traity.

    Co se týče metody createTemplate() jediné co mně na ní v současné době štve je, že je protected. Co se týče viditelnosti metod které používá interně, tak to je v podstatě jedno, protože pouze využívá vlastnosti jazyka. Také bych ji raději viděl ve zvláštní třídě a vznikly na to už nejméně dva pullrequesty, ale vždycky s tím byl nějaký problém.

    Být tebou, příště bych zkusil místo článku sepsat RFC na fórum a zkusit napsat ukázkovou implementaci, popř poslat pull. Nette používáš také, takže tím pomůžeš i sobě. Nevidím důvod, proč by měl dělat David úplně všechno sám.

  6. koubel - 23. 7. 2012 10:36

    [4 - Tharos][@HosipLan] - Tak to jsem rád, že nejsem sám, komu se současná dědičnost v komponentovém modelu nezdá a že dokonce existují aktivity směřující ke změně. Traity by možná ani nebyly třeba, jak se píše v tom Tharosem odkazovaném vláknu na fóru a roadmapě, nějaká kompozice objektů by to asi také zajistila. Ale určitě to není snadné. Na ukázkovou implementaci se tedy zrovna necítím a bez podstatné asistence D. Grudla se to určitě neobejde. Ta změna dědičnosti by pak určitě "samovolně" přinesla řešení všech ostatních věcí, které v článku zmiňuji.

    Ohledně protected method Presenteru - problém to u určitých metod je, konkrétně zmíněná metoda Presenter::getHttpRequest se v uživatelských komponentách odvozených od Control volat prostě nedá a nenapadá mě žádný důvod aby to byl záměr - proč bych ve své komponentě nesměl zavolat

    $this->getPresenter()->getHttpRequest()>
    Není podle mne žádný důvod, aby tato metoda nebyla public.

  7. David Grudl - 22. 8. 2012 20:45

    Ahoj a díky za článek. Se správným používáním dědičnosti je to někdy ošemetné. V Nette vím o případu, kdy je dědičnost použita vyloženě špatně, někteří dokonce dogmaticky odsuzují dědičnou jako takovou a nepoužili by jí nikde. Zrovna v případě UI\Presenter

  8. David Grudl - 22. 8. 2012 20:46

    Ahoj a díky za článek. Se správným používáním dědičnosti je to někdy ošemetné. V Nette vím o případu, kdy je dědičnost použita vyloženě špatně, někteří dokonce dogmaticky odsuzují dědičnou jako takovou a nepoužili by jí nikde. Zrovna v případě UI\Presenter < UI\Control vyloženě problém nevidím.

    Souhlasím, že hlavním úkolem presenteru není být skladem na komponenty, ale odbavit daný požadavek. Od toho slouží interface IPresenter, jehož je UI\Presenter jen jednou z implementací. Přímo ve frameworku existuje také nekomponentový presenter, lze vytvořit i vlastní implementace.

    UI\Presenter naopak skladem na komponenty je a je to jeho podstatná vlastnost. Možná by bylo lépe to realizovat namísto dědičností skládání, možná by se daly použít traits, nejsem si jist. Tehdy před 5 lety mi myšlenka, že i presenter je komponenta, připadala inspirativní.

    Ad protected getHttpRequest: viditelnost protected není správná, lépe by bylo private (asi byly historické důvody, proč je protected). Pokud komponenta potřebuje objekt HTTP požadavku, správnou cestou z hlediska DI je si o něj říct například v konstruktoru.

Komentáře jsou uzavřeny.