Nette\Object a řešení problému s method gettery

Nebylo by lepší udělat nějaký nástroj do laděnky (panelu dolu), kde se budou výskyty těchto konstrukcí ukazovat a nebo možnost spustit nějaký test. Potlačovat varovaní v kódu zavináčem mi přijde jako něco co by se mělo používat až jako poslední možnost.

A pokud přikročíš k tomu, že to opravdu bude vyhazovat varovaní, tak by bylo dobré mít možnost tuto kontrolu vypnout.

Editoval Lukes (29. 9. 2015 18:42)

Osobně method gettery používám pouze pro zpracování formulářů. Používám je převážně tímto způsobem (zjednodušený kód, ale snad to z něj půjde pochopit).

class FooFormFactory
{
    public function create() { return new Form(); }

    public function addFormSubmitted(Form $form) { }

    public function editFormSubmitted(Form $form) { }
}

class FooPresenter
{
    public $fooFormFactory;

    public function createComponentBarForm()
    {
        $form = $this->fooFormFactory();
        $form->onSuccess[] = $this->fooFormFactory->addFormSubmitted;
    }
}

Nicméně osobně si myslím, že by neměl být jakýkoliv BC break. Navíc, vzhledem k tomu, jak je automatické testování presenterů „otravné“ (a hlavně odesílání formulářů, apod.), tak kdyby někdo udělal composer update, spustil testy a myslel si, že je vše OK, byl by velmi nemile překvapen při deploynutí updatlé verze aplikace. :/

Nejpřijatelnější řešení je podle mě opt-in nastavení, které by tyto chyby začalo házet – klidně na všechny method gettery. V sandboxu můžeš hodit toto nastavení na true (nové projekty se této potenciální chybě vyhnou), ale existující projekty by to nijak neovlivnilo – pokud si sami nezvolí.

Osobně je používám na předávání callbacků metodám jako parametr metody.

<?php

	$this->angularPost($this->save, [$paramSave], $this->loadData, [$paramLoad]);

?>

Abych furt nemusel opisovat zpracování AngularJS požadavku.

@DavidGrudl No angularPost používám jako generickou funkci pro zpracování toho požadavku a metody save() a loadData() můžou být prakticky jakékoliv (většinou z modelu). Čili může se stát, že jsou bez parametrů, ale i s nepovinnými ale i povinnými.

Editoval Lukes (29. 9. 2015 21:04)

Rad bych to vyzkousel na projektu. Mam aktualne

"nette/nette":              "~2.3.4",
"nette/utils":              "2.3.*@dev",

nejaky tip, jak to upravit, aby to includlo dev utils?

Přijde mi škoda tuhle užitečnou featuru odebírat nebo nějak ořezávat… co takhle to řešit opačně, že u security-critical metod by se toto chování explicitně dalo zakázat? něco jako

/** @disableMethodGetter */
public function isLoogedIn() { ... }

Připadá mi, že obecně případů, kdy to ohrozí přímo bezpečnost, je velmi málo (metody dotazující se na ACL vyžadují parametry), resp. mě ani nenapadá jiný příklad, než isLoggedIn…

@DavidGrudl Jsem pro A), ale osobně to považuju za BC break, protože budu nejspíš muset na některé místa dopsat ty zavináče, abych s aplikací mohl pracovat i na lokálu. Je to pro mne úplně stejně otravná změna (byť asi nutná) jako pro tebe ty anotace od @amik.

@castamir zkus prohledat svůj kód na pattern /->(is|get|has)([A-Z]\w*)?[^\w(]/ a rychle zjistíš, jestli někde budeš potřebovat psát zavináč klasický PHP callback.

@PavelKravčík byl bych raději, kdybys to vyzkoušel, dokud je to v masteru, a pomohl to doladit.

Pavel Kouřil napsal(a):

Ano, co se týče specifického $user->isLoggedIn, tam jsem si téměř jistý tím, že se mě to netýká – neboť používám $user->loggedIn (a vůbec, velmi často používám magické properties).

Btw, taky jsem dost používal magic properties, dokud jsem nezjistil, že jsou klidně i 100× pomalejší, než přímé volání metody, podle mě je tohle perfektní nápad a jsem za variantu A).
@DavidGrudl keep going!

P.S. díky za regulár, ověřil jsem si, že tím damejidlo netrpí a sakra nechci aby kdy trpělo.

Editoval Jan Endel (30. 9. 2015 11:55)

David Grudl napsal(a):

Je totiž mnohem větší pravděpodobnost, že zápis $user->getIdentity je případem zapomenutých závorek, než že bych chtěl použít method getter.

Problém je, že validní použití prostě nepoznáš podle názvu. A zavináč používat nechceš nikdy, protože to je vícenásobný mistr světa na ukrývání chyb v kódu.

// tohle je například validní použití method getteru
$identityFactory = $user->getIdentity;
$shouldShowSomethingDecider = $user->isAuthenticated; // blbej název, vim, ale snad jde poznat co jsem myslel

Překlepy samozřejmě můžou nastat kdykoliv. Ale smysl bojovat proti nim to má tehdy, pokud to nepřináší nežádoucí vedlejší účinky. A pokud k těm chybám dochází častěji než k jiným. Máš to Davide podložené jinak než že se ti to jednou stalo?

Můj názor je ten, že pokud v javascriptu, ruby ani pythonu nemají to potřebu řešit, zřejmě se o nějak závažný problém nejedná.

@HonzaMarek problém je v tom, že třída Object, jejímž prapůvodním a dodnes IMHO primárním účelem je boj proti překlepům (zejména zápis do neexistující property), najednou zvyšuje riziko chyby oproti tomu, když by použita nebyla.

To je rozhodující (a dostačující) důvod s tím něco dělat.

Že neexistuje optimální řešení je snad jasné. Takže se skutečně hodně snažím najít alespoň to nejlepší možné. A divím se, proč namísto spolupráce se mnou bojujete nebo vysvětlujete zjevné.

@Lukes a není prostě jednodušší použít [$user, 'isAuthenticated'], než dopisovat anotaci, jen abych mohl zapsat $user->isAuthenticated?

Jde o to, aby existující kód fungoval bez warningů i bez zásahů. Pokud už bych zasahovat musel, tak bych method getter oželel.

Překvapuje mě vaše sebejistota, že nikde v šabloně nemáte <div n:if=„$user->isLoggedIn“> nebo cokoliv podobného.

Jsem si vcelku hodne jistej, protoze: mame IActor, kterej implementuje Guest a User. Bud to neprojde typehintem, nebo to velmi brzo skonci na tom, ze guest nema id. Tedy nebezpeci, ze se navstevnik dostane k necemu, k cemu nema, je naprosto minimalni.

<possible-offtopic>U Nette\Objectu by bylo fajn, kdyby se rozdělil na Object chránící proti překlepům a Object+, kterej by přinášel všechny nadstavbové vychytávky.</possible-offtopic>

Jan Tvrdík napsal(a):

Ve 2.3 bych manuálně zakázal, aby $user->isLoggedIn vracelo callback

a neuděláš tím větší BC break než je teďka? Co chudáci aplikace co na $user->isLoggedIn callback spoléhají? Není takových potenciálně nebezpečných metod mnohem víc?

Nikdo si aktivně nestěžoval…

Narazil jsem na to sám, tuhle mi o tom říkal třeba @FilipProcházka, příklady jsou i na fóru, například if (!$this->getUser()->isLoggedIn) https://forum.nette.org/…eslani-formu#….

stačí zakázat isLoggedIn

A co třeba if ($this->isAjax) https://forum.nette.org/…zpet-a-vpred#…, byť tehdy šlo o překlep? Na některé věci, jako například $control->getValue https://forum.nette.org/…tni-validace#…, se přijde rychle, ačkoliv chybová hláška musí být hodně matoucí zejména pro začátečníjky, ale co je zrada i pro ostřílené borce, je třeba foreach ($formular->getErrors as $input) https://forum.nette.org/…zit-validaci#…, protože iterování nad closure je korektní operace a důsledkem pak je, že se nevypisují chyby.

Jinak, zkusil jsem tedy tu 2.3.*@dev na 2 projektech a na žádné problémy v mých use-casech jsem nenarazil.

EDIT: Možná jsem měl prve ověřit, že se mi opravdu ta @dev stáhla … Zkusím to znovu zítra, snad již na správné verzi.

Editoval Pavel Kouřil (2. 10. 2015 21:50)