Jiné závislosti pro děděný objekt než pro rodiče

Jak píše @KamilValenta. Už samotné Translator extends BaseService rovná se špatný návrh kódu, bez šance to jakkoliv vysvětlit.

Lukáš Francálek napsal(a):

• Ve kterém místě se stává návrh špatným?
• Pokud mi u BasePresenter a ModelPresenter fungují – je to OK nebo už to je blbě? Přece ale nebudu předávat všem modelům stejné závislosti, ne?

Zhruba v okamžiku, kdy vytvoříš první třídu, která bude dědit Base* předka, ale nevyužije všechny jeho závislosti.
Na dlouhé zimní večery bych doporučil k pročtení toto vlákno – https://forum.nette.org/…bo-dedicnost

• Jak celý tento problém vyřešit tak, aby instance ColorService neběžela dokud nemá všechny své závislosti – obecné i specifické)?

Závislost vyžadovat v konstruktoru. Pokud nejsem schopen závislost dodat už při registraci služby v DI kontejneru, tak bych si napsal továrnu, která mi instanci vyrobí. Tovární metodě bych pak předal všechny parametry. A pokud bych potřeboval, aby všechny instance pro konkrétní hodnotu parametru byly identické, tak nevolat pokaždé new ColorService(), nýbrž si vytvořený objekt uložit do pole.

V kódu zhruba takto:

class ColorServiceFactory {
    private array $instances = [];

    public function create(int $serviceId): {
        if (isset($this->instances[$serviceId]) === false) {
            $this->instances[$serviceId] = new ColorService($serviceId);
        }

        return $this->instances[$serviceId];
    }
}

Továrnu zaregistruji v DI kontejneru, na místě, kde ji chci použít, si ji vyžádám přes konstruktor, a pak použiji $this->colorServiceFactory->create(25)->....

Lukáš Francálek napsal(a):

Protože jazyk je povinný, takže nechci řešit problém s tím, že není nastaven, ale instance třídy existuje.

V tomto konkrétním případě bych se nebál mít jeden jazyk jako výchozí a public metodou to případně jen přepnout na jiný.

Mě ty Base třídy vyhovují. Vždycky dělávám BasePresenter, BaseModel a těm definuji právě základní funkce, které chci pro všechny Presentery a Modely. To je ještě OK nebo už to je špatně?

No, toto vlákno vzniklo proto, že Ti to vlastně až tak moc nevyhovuje, protože v předkovi (Base*) máš závislosti, které dál nechceš.
Druhý neduh je, že programátor tu závislosti nevidí, dokud si předka neproklikne.
Třetí neduh je, pokud se nemýlím, že se takové závislosti nedají při testech mockovat (nebo alespoň já nevím jak).

• Pokud mi u BasePresenter a ModelPresenter fungují – je to OK nebo už to je blbě? Přece ale nebudu předávat všem modelům stejné závislosti, ne?

Proč ne?

• Jak celý tento problém vyřešit tak, aby instance ColorService neběžela dokud nemá všechny své závislosti – obecné i specifické)?

Já bych se nejprve snažil dostat z hlavy termín „obecná závislost“. Ony jsou všechny specifické :) Některé jsou méně časté, jiné více. Ale specifické jsou všechny.

Můžeš mi ukázat kód, kde ty služby voláš tak, že potřebuješ aby měly jednotné rozhraní? Mě nenapadá případ, kdy bych potřeboval jednotně volat metodu ze nějaké service a bylo mi jedno jestli mi vrátí Color nebo Gps.

Třída co počítá vzdálenosti mezi body by vůbec neměla sahat do databáze. TA by měly dostat dva body vytažené z databáze a spočítat vzdálenost.

Z toho co popisuješ mi fakt čím dál víc přijde, že ty používáš dědičnost místo kompozice, protože ti to přijde jednodušší než si předávat závislosti. Což ti pak přináší spoustu komplikací.

Ty bys měl mít třeba objekt Gps, který představuje souřadnice a nemá ideálně žádné závislosti a nějakou službu GpsRepository, která zajišťuje načtení/uložení Gps do databáze. Pak můžeš mít třídu GpsCalculator, která umí s předanými Gps objekty dělat výpočty.

Když pak chceš spočítat vzdálenost 2 bodů tak si jako závislost dáš GpsRepository, na něm zavoláš metodu find abys získal první a druhý bod, Pak si vytvoříš GpsCalculator, předáš mu ty dva body a necháš si spočítat výsledek.

Tak máš jasně rozdělené zodpovědnosti a oddělené závislosti. Takže pak případné změny a rozšiřování jsou mnohem jednodušší.

mystik napsal(a):

Můžeš mi ukázat kód, kde ty služby voláš tak, že potřebuješ aby měly jednotné rozhraní? Mě nenapadá případ, kdy bych potřeboval jednotně volat metodu ze nějaké service a bylo mi jedno jestli mi vrátí Color nebo Gps.

Kód není, ale je to tak, jak říkám :D
Lepší příklad – převody jednotek…

Kdybych dělal web na převody jednotek. V DB bych měl 100 kombinací převodů a podle requestu uživatele bych zvolil správnou službu (Vzdálenost/Objem/Rychlost) a předal jí data uživatele (from:‚km‘, to:‚m‘, value:5) a služba by si třeba sáhla do DB (a nebo ne – to je její věc) a vrátila „5000“. To bych poslal uživateli.
V Presenteru je mi jedno jestli jde o vzdálenost nebo objem – vezmu data od uživatele, dám službě a návratová data ze třídy vrátím uživateli. Jen musím zavolat správnou službu třeba DistanceConverter implementující IConverter a dědící z BaseConverter, která má třeba metodu array getData(void), která jen: return $this->result;

Tu kompozici tady nevidím jako přínos. Která třída by vlastnila kterou? Kdybych měl třídu Converter, která má přístup k DB a ta by vlastnila třídu Distance, tak jí stejně musím předávat ty přístupy do DB, protože nevím, jaká data chce (a jestli vůbec).

Seriál přečtu, dík.

interface Converter
{
    public convert(float $value, string $fromUnit, string $toUnit): float;
}

$converter = new DatabaseConverter($db);
echo $converter->convert(10, 'km', 'm');

Pokud bys chtěl data reprezentovat objektově pak třeba:

interface ValueWithUnit {
    public function getValue(): float;
    public function getUnit(): string;
    public function __toString(): string;
}

class Distance implements ValueWithUnit {

}

interface Converter
{
    public convert(ValueWithUnit $from, string $toUnit): ValueWithUnit;
}

$converter = new DatabaseConverter($db);
echo $converter->convert(new Distance(10, 'km'), 'm');

Pak si můžeš udělat jakoukoli implementaci converteru. Přes databázi, přes soubor, přes volání API, … a zbytek kódu bude úplně stejný. Pokud začneš databázi míchat někam do vstupů, výstupů tak si zaděláváš na problém.

Editoval mystik (11. 1. 2023 20:49)

Lukáš Francálek napsal(a):

@nightfish Pokud mám třeba 30 služeb se stejným interfacem a většina připojení k DB použije, tak považuješ za lepší mít 30 továren a zvlášť závislosti? Mě přijde lepší jeden předek s přístupem do DB a když třeba 7 z těch 30 služeb DB nepotřebuje, tak .. mě to moc vadit nebude, ne?

Mám 30 služeb, z toho tak 25 má závislosti, které jsou „pevné“ – službu zaregistruji do DI kontejneru, továrnu nepotřebuji. Některé z těch služeb nemají žádné závislosti, některé potřebují databázi, některé potřebují cache, některé potřebují jiné služby – do každé služby dám jenom ty závislosti, které potřebuji. (Když tam mám nějakou navíc, tak mě tuším PHPStan upozorní, že mám ve třídě property, do které se jen zapisuje – to je závislost, kterou v dané třídě vůbec nepoužívám a mohu ji tedy odstranit.)
Pro zbývajících 5, kterým potřebuji závislost přidat runtimově, buď udělám Nette generovanou továrnu (interface + registrace do DI) – pokud potřebuji vytvářet více instancí této služby, a nebo napíšu tovární třídu (class + registrace do DI), která vytvoří instanci jenom jednou a při dalších voláních mi tuto instanci vrací opakovaně.

Pokud bych měl 30 služeb a k nim potřeboval 30 továren, tak by mi to přišlo zvláštní a snažil bych se vymyslet, jestli by to nešlo udělat jednodušeji.

>

Rady nezpochybňuji, vážně si chci dostat do hlavy vaše myšlení a zkušenosti. Jinak za ukázku díky. Nelíbí se mi mít tolik továren kolik mám služeb, ale umím si představit $service = ServiceFactory->create(25, 'ColorServiceFactory').

Volat továrnu, které jako argument předáš továrnu, kterou to má použít k vytvoření něčeho, sice možné je, na pár projektech to používáme, ale z vývojářského hlediska je to velmi nepříjemné, protože to vytváří skryté závislosti, které z kódu ani konfigurace DI kontejneru napřímo nevyčteš.

@mystik K tomu příkladu s převodem…
A teď hypoteticky … řekněme, že mám délkovou míru loket a značí se „l“ a mám litr, který je taky „l“. A řekněme, že délkové míry se převádí jiným způsobem než objemové. Té metodě convert bych musel předat i info o jakou jde veličinu aby si mohla zjistit, jestli požaduji vyřešitelnou kombinaci a jak ji vyřešit. A jednotlivá řešení by byla kde? Určitě ne v metodě convert, ale v jednotlivých třídách reprezentující veličiny, ne?
Tzn. naznačuji potřebu různých výpočtů, ale se stejným rozhraním (vstupem a výstupem).

A ještě poprosím o reakci na:

Lukáš Francálek napsal(a):
Ale obecně – nejde jen o připojení k DB, ale když si udělám BaseModel a ten má připojení k DB a nějaké obecné metody pro práci s DB, třeba findAll() a findById() atp. a ošetřené názvy DB tabulek (nerad používám hardkódnuté názvy tabulek pro případ změny názvu nebo potřeby zavést prefix atp.).
A z něj dědí třeba ArticleModel, kde neřeším NIC obecného – jen používám $this->db a $this->tableName a $this->findAll() a až když potřebuji, tak přidám metodu findByAuthor(). Toto je fakt špatný přístup?

No a pokud by toto ještě bylo OK, tak co když do BaseModelu předávám krom DB i Cache?

Protože Modely pracují s daty z DB a mohou chtít načtená a zpracovaná data cachovat. Mohl bych předávat Cache, jen tam, kde ji fakt potřebuji (ale už bych bojoval s tím, že BaseModel chce DB a ArticleModel extends BaseModel chce i Cache a už by se to komplikovalo v konstruktorech.

Proto dám Cache už BaseModelu a mají ji všichni nehledě na to, jestli některé modely Cache nepoužijí. A opět – udělám si třeba pomocné vlastnosti nebo metody pro cache přímo v BaseModelu – např. $this->getCache(?string $key = null), kdy pokud nepředám $key, tak se vytvoří cache s klíčem podle názvu děděné třídy, takže ArticleModel->getCache() dostane cache s klíčem ArticleModel.

Je mi jasné, že tady už je něco špatně, ale nevím co :D
Resp. trošku vím, ale spíš nevím, jak z toho ven…

Editoval Lukáš Francálek (13. 1. 2023 9:48)

Třeba ten příklad s cache bych asi řešil takhle

abstract class BaseCachedModel extends BaseModel {

    public function __construct(Database $db, Storage $cacheStorage)
    {
        parent::__construct($db);
        $this->cache = new ModelCache($cacheStorage, static::class /* default key */);
    }
}

Editoval mystik (13. 1. 2023 9:57)

Volat konstruktor rodiče přece není žádný problém ne? Tomu by ses neměl snažit vyhýbat. Naopak tohle bys měl upřednostnit před čímkoli jiným.

Předpokládám, že se bojíš situace, kdy rodiči přidáš závislost a budeš muset upravit všechny potomky. Pokud ti tohle hrozí je to opět známka, že bys možná měl změnit strukturu. A závislosti rodiče nějak sdružit. Viz v mém příkladu Modelu předávám jen Mapper. Pokud přidám cache do Mapperu Model ani jeho potomci se nezmění.

V okamžiku, kdy máš pocit, že bys potřeboval multidědičnost jsi právě v bodě, kde začínáš vidět, proč je lepší preferovat kompozici před dědičností. Při kompozici si můžeš libovolně nakombinovat jaké objekty používáš.

To že máš třídu, která toho umí hodně a od ní dědíš věci, které dělají jen specifika je chyba. Ta třída totiž bude nevyhnutelně bobtnat a jakákoli změna v ní bude ovlivňovat spoustu jiných tříd. Dlouhodobě je to pak noční můra na udržování. Opravdu radši vol kompozici. Několik malých tříd, které dělají to co máš teď v tom BaseModel. A jednotlivé modely si jen řeknou o ty z nich, které používají.

Zapomenute volani konstruktoru rodice je bug ktery odhalis hned.

Pokud jenzavislosti moc je to signal ze je neco spatne a mel bys mozna nejakou funkcnost presunout do oddelene tridy nebo tridu rozdelit.

ServiceLocator nepouzivat nikdy.Mapper je proste trida co zapouzdruje presistenci dat. Umi predany objekt ulozit/nacist z databaze, hledat,…

Na co narazis u kompozice za problem? Muzse hodit priklad kde nevis jak to pouzit? Zkusim ti ho upravit.

Ty predpokladas ze funkcnost je bud v rodici nebo rozkopirovana v potomcich. Ale lepsi reseni je ji nemit ani v rodici ani v potomcich ale v samostatne tride, kterou pak pouzivaji potomci co tu funkcnost potrebuji. Dedeni funkcnosti od rodice bys mel nahradit kompozici.

Zjus sem hodit kod jak tu db a cache v base modelu pouzivas

BaseModel

declare(strict_types=1);

namespace App\Model;

use Nette\Caching\Cache;
use Nette\Caching\Storage;
use Nette\Database\Explorer;
use Nette\Database\Connection;
use Nette\Database\Table\Selection;


abstract class BaseModel
{
	protected string $tablePrefix = '';
	protected string $defaultTableName = '';
	protected array $tables = [
		'article' => 'article',
		'author'  => 'author',
	];

	public function __construct(
		protected Connection $dbConnection,
		protected Explorer $dbExplorer,
		private Storage $cacheStorage,
	) {
	}

	public function findAll(): Selection
	{
		return $this->dbExplorer->table($this->getTableName($this->defaultTableName));
	}

	protected function getTableName(?string $tableName = null): string
	{
		$tableName = $tableName ?? $this->defaultTableName;
		return $this->tablePrefix . isset($this->tables[$tableName]) ? $this->tables[$tableName] : $tableName;
	}

	protected function getCache(?string $nameSpace = null): Cache
	{
		$nameSpace = $nameSpace ?? $this->getClassName();
		return new Cache($this->cacheStorage, $nameSpace);
	}

	protected function getClassName(): string
	{
		$classParts = explode('\\', get_class($this));
		return end($classParts);
	}
}

Odvozený model

declare(strict_types=1);

namespace App\Model;

use Nette\Database\Table\ActiveRow;


final class ArticleModel extends BaseModel
{
	protected string $defaultTableName = 'article';

	public function findBySlug(string $slug): ?ActiveRow
	{
		$article = $this->dbExplorer
			->table($this->getTableName())
			->where('slug = ?', $slug)
			->fetch();
		return $article;
	}
}

Což u těch modelů nemám problém, pokud mi nevadí, že mohu mít v nějakém modelu závislost, kterou pak nepoužiji. Problém je, když ten BaseModel mám tendence dát jako předka jiné třídě, která už má jinou funkčnost, ale taky potřebuje DB a Cache a ty bázové metody – ale navíc vlastní konstruktor. To už cítím, že scházím z pěkné cesty…

Editoval Lukáš Francálek (14. 1. 2023 10:01)

Zhruba něco v tomhle smyslu:

abstract class BaseMapper
{
	public function __construct(
		string $tablePrefix,
        protected string $defaultTableName,
		protected Connection $dbConnection,
		protected Explorer $dbExplorer
	) {
	}

	public function findAll(): Selection
	{
		return $this->dbExplorer->table($this->getTableName($this->defaultTableName));
	}

	protected function getTableName(?string $tableName = null): string
	{
		$tableName = $tableName ?? $this->defaultTableName;
		return $this->tablePrefix . isset($this->tables[$tableName]) ? $this->tables[$tableName] : $tableName;
	}
}

abstract class BaseModel
{
	public function __construct(
		protected Mapper $mapper,
		private Storage $cacheStorage,
	) {
	}

	public function findAll(): Selection
	{
		return $this->mapper->findAll();
	}
}

abstract class AnotherService
{
	public function __construct(
		protected Mapper $mapper,
	) {
	}

	public function findAll(): Selection
	{
		return $this->mapper->findAll();
	}
}

`

Editoval mystik (14. 1. 2023 9:55)

abstract class ArticleMapper extends BaseMapper
{
	public function __construct(
		protected Connection $dbConnection,
		protected Explorer $dbExplorer
	) {
        parent:.__construct('article', '', dbConnection, dbExplorer);
	}

}

abstract class BaseModel
{
	public function __construct(
		protected ArticleMapper $articleMapper,
		private Storage $cacheStorage,
	) {
	}

	public function findAll(): Selection
	{
		return $this->articleMapper->findAll();
	}
}

Editoval mystik (14. 1. 2023 9:57)

To je velke dilemma jak udelat tuhle strukturu. Nette doporucuje spis podle modulu (skupina temat) a pak podle funkce. Me spis vyhovuje podle funkce a az v dalsi urovni podle modulu (u jednoduchy apek tuhle vrstvu nekdy vynecham). Ale je to spsi otazka co se ti vic libi nez ze by na to byla spravna odpoved.

Pokud bys chtel na tohle tema dalsi zdroje tak muzu doporucit nejake knihy/clanky.