je ModelLoader k vůbec něčemu?

Neas

Ahoj.
Napsal jsem si zrovna svůj ModelLoader, uvedl jsem ho v configu a vše funguje, jak má. Prohlížím si ale právě zdroják ProfilePresenteru a něco mě zarazilo:

public function renderView($id)
{
    $this->template->profile_info = $this->getModel('MyApp\Security\Profile')->init($id);
    $this->template->comments = $this->getModel('MyApp\Security\Profile')->wall->comments;
    $this->template->commentsCount = $this->getModel('MyApp\Security\Profile')->wall->Count();
}
protected function getModel($model)
{
    return $this->context->modelLoader->loadModel($model);
}

Říkám si, jestli by nebylo jednodušší, napsat v renderovací metodě prostě přímo:

public function renderView($id)
{
    $profile = new MyApp\Security\Profile($id);
    $this->template->profile_info = $profile->info;
    $this->template->comments = $profile->wall->comments;
    $this->template->commentsCount = $profile->wall->Count();
}

O veškeré záležitosti modelu se dál starají jiné třídy, než presenter, takže je zachován koncept MVC, kód se prakticky nezměnil, jen vypadla celá třída ModelLoader a navíc je s tím míň psaní.

Rád bych znal vaše názory,
děkuji

Editoval Neas (11. 4. 2012 0:07)

Vojtěch Dobeš

Jasně že je to lepší :) Ještě lepší by bylo si zaregistrovat tvou třídu MyApp\Security\Profile pod nějakým názvem jako službu v konfiguračním souboru (config.neon). Třeba pod názvem securityProfile.

services:
	securityProfile:
		class: MyApp\Security\Profile

A v presenteru volat $profile = $this->context->securityProfile;.

A ještě lepší by bylo přidat presenteru setter:

public function setSecurityProfile(\MyApp\Security\Profile $profile)
{
	$this->securityProfile = $profile;
}

A při vytvoření presenteru tuto metodu zavolat s patřičnou službou předanou autowiringem – ale jak na to nechám na zodpovězení zkušenějším, sám jsem to totiž nikdy nedělal. To by byla pravá propečená DI – presenter by dostal přesně to, co využije (tedy instanci MyApp\Security\Profile), a nestaral by se, kde to získá (teď totiž presenter musí vědět, že si pro instanci MyApp\Security\Profile má šáhnout do $this->context).

Editoval vojtech.dobes (11. 4. 2012 0:39)

Filip Procházka

Né ne, byť netuším, co ta třída dělá, tak daleko lepší by bylo.

// protože použiješ nativní implementaci, IDE ti bude napovídat
$profile = $this->context->myApp->securityProfile->find($id);

$this->template->profile_info = $profile;
$this->template->comments = $profile->wall->comments;
$this->template->commentsCount = $profile->wall->Count();

Úplně tam totiž cítím, jak v té třídě lovíš něco ze vzduchu!

Editoval HosipLan (11. 4. 2012 7:52)

arron

@neas: @vojtech.dobes ma naprostou pravdu a přesně takhle by to mělo vypadat v ideálním světě (kódu). Bohužel injectování služeb do presenteru se zatím tak upně nedá…
Oba Tvé kusy kódu vykazují jisté nedostatky. Ten druhý se zpronevěřuje principům DI a tuhle úpravu bych rozhodně nedělal, je IMHO krokem zpět. Ten první se zpronevěřuje zásadám psaní „čistého kódu“ a to konkrétně tak, že se Ti tam Tvůj kód opakuje několikrát (duplicita kódu). V Tvém konkrétním případě doporučuji refaktorovat alespoň následovně:

public function renderView($id)
{
    $profile = $this->getProfileModel();
    $this->template->profile_info = $profile->init($id);
    $this->template->comments = $profile->wall->comments;
    $this->template->commentsCount = $profile->wall->Count();
}
protected function getProfileModel() {
    return $this->getModel('MyApp\Security\Profile');
}
protected function getModel($model) // tahle metoda patří asi do nějakého BasePresenteru
{
    return $this->context->modelLoader->loadModel($model);
}

ModelLoader Ti v tomto případě značně ulehčuje práci, protože nemusíš každý model zapisovat do konfigurace, ale když ho potřebuješ, tak si ho prostě vytáhneš (a případně injektuješ jako závislost někam jinam). Pokud máš těch modelů pár (jednotky), tak to až takový rozdíl nění, pokud jich máš víc, tak pak už je to pruda to všechno vypisovat do configu (krom toho, že ten config je pak nepřiměřeně dlouhý).