Autor Zpráva
pcmanik
Profil
Zdravím,
Začínam s OOP a rád by som sa na jednoduchom príklade opýtal, či to robím správne, resp. ako to robiť inak - lepšie.

Povedzme, že užívateľ vytvára skupinu. Ako ma vyzerať správny zápis?
V súčastnosti mám
$group = new Groups;
$group->name = Protect::input($_POST['nazov']);
$group->description = Protect::input($_POST['popis']);
$group->visibility = intval($_POST['viditelnost']);

if ($group->validateCreateGroup() === false) $data = $group->error;
elseif ($group->nameExists()) $data = array( 'status' => 'ERR', 'result' => 'Skupina s rovnakým názvom už existuje.' );
elseif (!$group->createGroup()) $data = array( 'status' => 'ERR', 'result' => 'Skupinu sa nepodarilo vytvoriť.' );
elseif (!$group->addAdmin($_SESSION['id'])) $data = array( 'status' => 'ERR', 'result' => 'Skupinu sa nepodarilo vytvoriť.' );
else $data = array( 'status' => 'OK', 'result' => $group->group );
Avšak je potrebné to riešiť takto pomocou metód? Alebo stačí jedna metóda v tvare:
$group->createGroup(array(
 'name' => $_POST['nazov'],
 'description' => ...
));
Ktorá sa o všetku logiku postará a pri úspechu vráti už vyšsie spomenuté pole status => OK resp. pri neúspechu status => ERR s chybovou hláškou?
Ak by to bolo len pomocou tejto jednej metódy, v jej vnútri mám používať ostatné spomenuté metódy ako addAdmin atď. Alebo jednoducho napísať všetok kód za sebou?
Overovanie vstupných premenných mám robiť ešte v controlleri, alebo až v modeli?

Ďakujem za každú odpoveď
Jan Tvrdík
Profil
pcmanik:
Asi bych začal připomenutím Single responsibility principle, tedy že každá třída by měla mít právě jednu zodpovědnost a tato zodpovědnost by měla být vyjádřena názvem té třídy. V tomto konkrétním případě by se tedy třída představující právě jednu skupinu měla jmenovat Group, nikoliv Groups. Stejně tak by neměla mít spoustu dalších zodpovědností jako ověřování existence skupiny, vlastní založení apod.

Teď postupně k jednotlivým řádkům:
ad Protect::input – Pokud to dělá něco jako mysqli_real_escape_string, tak to není ideální, protože objekty by pokud možno měly obsahovat surová data a escapovat by se mělo až při konkrétním použití. Pokud ta metoda např. kontroluje, že se jedná o validní UTF-8 řetězec, tak je to v pořádku.

$group->nameExists() – Připomínám, že tohle je špatné řešení z pohledu atomicity. Nelze zaručit, že mezi voláním nameExists() a createGroup() nebude v jiném vlákně / procesu vytvořena jiná skupina se stejným názvem. Správné řešení je pokusit se skupinu vytvořit a v případě selhání zkontrolovat chybový kód podle kterého lze selháním na unikátním klíči detekovat.

ad zpracování chyb obecně – Na tohle se více hodí výjimky. Vypadalo by to pak třeba takto:

try {
    $group->createGroup();
    $group->addAdmin(...);
} catch (GroupNameAlreadyExistsException $e) {
    $errMsg = 'Skupina s rovnakým názvom už existuje.';
} catch (GroupNotValidException $e) {
    // ...
} catch (GroupCreationFailed $e) {
    // ...
}

ad $data = $group->error; Tímhle navíc porušuješ Separation of concerns, protože metoda validateCreateGroup() by ideálně neměla tušit nic o tom, že výstupem stránky je HTML ve slovenštině.

ad $group->createGroup(...) Nedává smysl říkat objektu Group, aby vytvořila skupinu. To by měla dělat spíš třída GroupFactory, nicméně v tvém případě asi spíš hledáš něco jako GroupsManager. U té metoda createGroup() (resp. jen create()) už smysl dává. S tím, že bude vyhazovat výjimky v případě selhání, pochopitelně.

v jej vnútri mám používať ostatné spomenuté metódy ako addAdmin atď. Alebo jednoducho napísať všetok kód za sebou?
Určitě rozdělit do více metod.

Overovanie vstupných premenných mám robiť ešte v controlleri, alebo až v modeli?
Controller i model by měly své vstupy kontrolovat, ale každý z jiného pohledu. Controller se dívá na požadavek jako celek, tedy kontroluje, že dorazil korektní POST požadavek se všemi očekávanými proměnnými a správnými datovými typy (ideálně včetně kontroly řetězců na validitu vůči UTF-8). Model poskytuje controlleru jasně definované API. Je zodpovědností controlleru toto API za každou cenu dodržet. Controller tedy obecně musí provést všechny kontroly, aby mohl garantovat respektování API modelu. Model pak rozumí datům více do hloubky a je-li to potřeba, může provést další kontroly.
pcmanik
Profil
Jan Tvrdík:
Ďakujem veľmi si mi pomohol a poučil. Protect::input v súčastnosti nerobí nič iné len orezáva reťazec pomocou funkcie trim. Vo všetkých dotazoch na db využívam prepared statements.

Takže keď samotná trieda nemá overovať existenciu skupiny, jej vytváranie a ani validáciu dát, za čo má byť teda zodpovedná?

Ako by teda mala byť správne napisaná ta validácia, tak aby nevedela v akom jazyku je výsledná stránka? A by som totiž niekedy v budúcnosti pridal viacero jazykov, tak len zmením stringy na polia v ktorých bude podľa jazyka obsah.

Dá s niekde v ucelenom návode zohnať viacej informácii o tom čo popisuješ, ako GroupFactory, GroupManager atď? Čo som doteraz našiel bolo neaktúalne, nekvalitné, alebo inak zlé. Ja som mal totiž za to že vytvorím triedu ktorá bude obsahovať všetky potrebné metódy na správu skupiny, či už jej vytvoranie, zmazanie, upravovanie, pridávanie členov atď.

Takže v controlleri mám len zistiť, či dorazili všetky premenné, ktore očakávam a o ďalšiu validáciu typu dĺžky reťazcov, či sa jedná o email atď sa má postarať model?
juriad
Profil
pcmanik:
Tak to vidím já; mnozí se mnou nebudou souhlasit.

Třída modelu má být zodpovědná jen za reprezentaci dat. Často je to jen seskupení atributů s hromadou getrů a setrů. Jelikož by (často) měl být model maximálně nezávislý na konkrétním úložišti (databáze, soubor, webová služba), nemůže obsahovat rozumnou implementaci vyšší logiky (například zjistit, zda taková skupina již existuje).

Veškerou logiku by měl obsahovat controller. Ten poskytuje rozhraní, které popisuje všechny operace, které umí provést bez ohledu na úložiště modelu.
Přikladem manageru je (v Javě, abych popsal typy):
interface IGroupManager {
  Group getGroupById(int id) throws GroupNotFoundException;
  Group createGroup(Group template) throws GroupAlreadyExistsException, GroupNameTooLongException;
  void addMemberToGroup(Group group, Member member) throws MemberAlreadyInGroupException, GroupFullException;
  void setEnabledStateOfGroup(Group group, boolean enabled);
}

GroupManager již musí dostat jen validní data – taková, která jsou konzistentní. Například, že skupina musí obsahovat jméno (přestože jméno GroupManager ke své práci nepotřebuje), formát emailu, záladní validace délek (podrobnější validaci může provést i model).
Až konkrétní implementace Manageru v rámci controlleru se stará o escapování (pro konkrétní databázi; pro URL, pokud je úložištěm webová služba, …).

Já bych proto ještě rozlišil vrstvičku Adaptéru před controllerem provádějící validaci, která zajistí, že pokud uživatel chce vytvořit novou skupinu, musí obsahovat vyplněná všechna nutná pole, že uživatel má dostatečná oprávnění k provádění operací a další výše popsané kontroly.
Zároveň instanciuje (či jinak získá) správný typ manageru controlleru a volá jeho metody (nejčastěji jen jednu).
Dobře se tato vrstva představuje v případě, že server se stará o vyřizování například jen JSON requestů. (prezentace tam není v podstatě žádná, ale validace tam třeba je)

A to celé je obalené prezentační vrstvou, která na základě typu požadavku vyrendruje stránku podle šablony a za pomoci dat získaných voláním adaptéru.

Existuje jediné místo (index.php), které odbavuje všechny požadavky; instanciuje adaptér; instanciuje prezentační šablonu pro daný typ requestu a předá jí adaptér.

Nějaké motivy a důvody separace vrstev můžeš vidět na diagramech tady: http://www.oracle.com/technetwork/java/dataaccessobject-138824.html
Tuto problematiku je lepší studovat bez omezení na konkrétní jazyk.
Jan Tvrdík
Profil
pcmanik:
Takže keď samotná trieda nemá overovať existenciu skupiny, jej vytváranie a ani validáciu dát, za čo má byť teda zodpovedná?
Vzhledem k tomu, že stavíš poměrně jednoduchou aplikaci s minimem vrstev, tak s velkou pravděpodobností třídu Group vůbec nepotřebuješ a vystačíš si s něčím jako GroupsManager, které předáš asociativní pole.

Pokud přeci jenom třídu Group měl, tak by měla roli entity (podle Martin Fowler spíš Domain Model). Entita se většinou používá jako chytřejší asociativní pole – zná svoji strukturu, dokáže validovat data, které obsahuje, zná svůj stav, ví jaké položky se změnili, může obsahovat (spíš jednoduchou) vlastní část business / doménové logiky.

Ako by teda mala byť správne napisaná ta validácia, tak aby nevedela v akom jazyku je výsledná stránka?
Většinou stačí správně používat výjimky a napsat si nějaký „translator“, který dokáže z informací, které validační výjimka obsahuje sestavit chybovou hlášku pro uživatele.

Dá s niekde v ucelenom návode zohnať viacej informácii o tom čo popisuješ, ako GroupFactory, GroupManager atď?
Tak jako můžeš si přečíst třeba Návrhové vzory od Rudolfa Pecinovského a P of EAA of Martin Fowler.

Ja som mal totiž za to že vytvorím triedu ktorá bude obsahovať všetky potrebné metódy na správu skupiny, či už jej vytvoranie, zmazanie, upravovanie, pridávanie členov atď.
To je v podstatě v pořádku, ale zodpovědnost té třídy by měla být vyjádřena v jejím názvu.

Takže v controlleri mám len zistiť, či dorazili všetky premenné, ktore očakávam a o ďalšiu validáciu typu dĺžky reťazcov, či sa jedná o email atď sa má postarať model?
Zjednodušeně řešeno – ano. V praxi se ale validace občas přesouvá (i když je to svým způsobem špatně) do controlleru.


juriad:
Třída modelu má být zodpovědná jen za reprezentaci dat.
Jelikož by (často) měl být model maximálně nezávislý na konkrétním úložišti (databáze, soubor, webová služba)
Veškerou logiku by měl obsahovat controller.
Pokud pod termínem „model“ myslíš to „M“ z MVC, tak máš o něm vyloženě špatnou představu. Model je vrstva aplikace, která tvoří datový a funkční základ aplikace. Tedy model by měl obsahovat veškerou business / domain logiku aplikace. Model nikdy nemůže být nezávislý od konkrétního úložiště, protože obsahuje práci právě s tím daným úložištěm. Rozdělíme-li model do více vrstev, tak některé z vrstev modelu mohou tuto nezávislost získat. Naopak controller (ve významu vrstvy aplikace) by měl vždy být zcela nezávislý na konkrétním úložišti.
juriad
Profil
Jan Tvrdík:
Aha, díky za články. Nikdy jsem to nestudoval, jen jsem se to snažil odpozorovat a popsat.

Koukám, že dohromady můj controller a model tvoří Model z MVC a můj adpatér tvoří Controller z MVC. To je také důvod proč jsem cítil potřebu zavádět další vrstvu adaptéra.

To rozdělení mi nejspíš vzniklo z důvodu používání GWT a Slim3 frameworku. Tam jako model je označovaná jen datová část, je sdílená mezi serverem a klientem; nemůže obsahovat žádnou logiku protože třídy/metody manipulující s databází nelze překompilovat do JS. Package model obsahuje jen entity (z pěti vrstevného modelu).
pcmanik
Profil
Jan Tvrdík:
Ďakujem za rozsiahle rady a poznatky, kopu vecí som sa priučil a idem študovať odkázané články.
Jedná sa o pomerne rozsiahlu aplikáciu, skupiny su len jedna z jej funkcionalít.
Jan Tvrdík
Profil
pcmanik:
Na závěr ještě doplním odkaz na výborný článek o architektuře aplikací od Vaška Purcharta. Sice je to pravděpodobně o dost složitější, než to, co momentálně potřebuješ, ale aspoň ti to dá vizi do budoucna.

Vaše odpověď

Mohlo by se hodit


Prosím používejte diakritiku a interpunkci.

Ochrana proti spamu. Napište prosím číslo dvě-sta čtyřicet-sedm: