Předávání factory do vytvořeného objektu

Michwuanquana
Člen | 21
+
0
-

Zdravím,

chci se zeptat, zda-li je správné předávat samotnou factory do vytvořeného objektu, odkud těží závislosti. Původně jsem jednotlivé třídy předával ručně do konstruktoru, pak jsem si řekl, zda by nebylo lepší předat přímo samotnou factory a ve vytvořeném objektu si třídy vytáhnout.

MailerFactory.php

class MailerFactory
{
	public array $config;
	public TemplateFactory $templateFactory;
	public LinkGenerator $linkGenerator;
	public SmtpMailer $sender;
	public Request $request;

	public function __construct(
		array $config,
		TemplateFactory $templateFactory,
		LinkGenerator $linkGenerator,
		SmtpMailer $sender,
		Request $request
	)
	{
		$this->config = $config;
		$this->templateFactory = $templateFactory;
		$this->linkGenerator = $linkGenerator;
		$this->sender = $sender;
		$this->request = $request;
	}

	public function create(string $mailClass): Presets\AbstractMail
	{
		$lang = preg_match('/^\\/([a-z]{2})\\//', $this->request->getUrl()->path, $matches) ? $matches[1] : Translator::DEFAULT_LANG;
		// tady jsem původně předával samotné templateFactory, linkGenerator a sender
		$mail = new $mailClass($this, $lang);//, $mailer);
		return $mail;
	}
}

AbstractMail.php

abstract class AbstractMail
{
	public const TEMPLATE_DIR = __DIR__ . '/../templates/';
	public const TEMPLATE_EXT = '.latte';

	protected LinkGenerator $linkGenerator;
	protected TemplateFactory $templateFactory;
	protected Message $message;
	protected SmtpMailer $sender;
	protected string $lang;
	protected array $config;

	public function __construct(
			MailerFactory &$mailerFactory,
			string $lang
	)
	{
		$this->templateFactory = $mailerFactory->templateFactory;
		$this->linkGenerator = $mailerFactory->linkGenerator;
		$this->sender = $mailerFactory->sender;
		$this->config = $this->config;
		$this->lang = $lang;

		$this->message = new Message;
		$this->message->setFrom($config['fromMail']);
	}

	protected function createTemplate(string $file): Template
	{
		$template = $this->templateFactory->createTemplate();
		$template->setTranslator(new Translator($this->lang));

		$template->getLatte()->addProvider('uiControl', $this->linkGenerator);
		$template->setFile(self::TEMPLATE_DIR . $file . self::TEMPLATE_EXT);
		return $template;
	}

	public function to(string|array $recipients): self {
		if(!is_array($recipients)) {
			$recipients = [$recipients];
		} else {
			if(!count($recipients))
				throw new InvalidArgumentException("Recipient is an empty array.");
		}

		foreach($recipients as $recipient) {
			if(!Validators::isEmail($recipient))
				throw new InvalidArgumentException ("$recipient is not valid email. All emails was not sent.");
		}

		foreach ($recipients as $recipient) {
			$this->message->addTo($recipient);
		}

		return $this;
	}

	public function send(bool $dump = false): void {
		if($dump)
			Debugger::dump($this->message);
		else
			$this->sender->send($this->message);
	}

	abstract public function build(mixed $data = null): self;
}

k tomu třeba univerzální BaseMail.php

class BaseMail extends AbstractMail
{
	public function build(mixed $data = null): self
	{
		$file = is_string($data) ? $data : $data[0];
		$template = $this->createTemplate($file);

		if(!is_string($data) && count($data) > 1) {
			unset($data[0]);

			foreach($data as $key => $value) {
				$template->$key = $value;
			}
		}

		$html = $template->renderToString();
		$this->message->setHtmlBody($html);
		return $this;
	}
}

Poradíte někdo prosím? Nemám zrušit AbstractMail a vše dát do factory?

Editoval Michwuanquana (4. 5. 23:24)

dakur
Člen | 351
+
+1
-

chci se zeptat, zda-li je správné předávat samotnou factory do vytvořeného objektu, odkud těží závislosti

Správné asi to není to správné slovo :-), ale určitě to není doporučené. Možná ti to teď přijde elegantní, jak ušetřit ruční předávání 5 proměnných, ale zkušenost praví, že si tímto patternem v dlouhodobějším horizontu spíš zavaříš (např. tím, že ti vzniká cyklická závislost). My někdy předáváme i 8 závislostí a není to zas takový problém, jednou to napíšeš a je to.

Spíš bych se ale zamyslel nad tím abstract a base mail. Opravdu potřebuješ třídu per e-mail? Nestačí nějaký obecný mail builder, do kterého bys jen vložil šablonu a parametry? Např.:

final class MessageBuilder
{
	public function __construct(
		private LinkGenerator $linkGenerator,
		// ostatní závislosti...
	) {}

	public function build(mixed $data): Message
	{
		// tady vytvoříš komplet zprávu, na vstupu máš data jako template file a parametry

		$message = new Message(); // ...
		$this->templateFactory->create(); // ...
		// ...
		return $message;
	}
}

To bys pak použil buď z komponenty, nebo z nějaké další service takto:

final class Mailer
{
	public function __construct(
		private Mailer $sender,
		private MessageBuilder $messageBuilder,
	) {}

	public function send(mixed $data, bool $dump = false): void
	{
		if ($dump) {
			Debugger::dump($this->message);
			return;
		}

		$this->sender->send($this->messageBuilder->build($data));
	}
}

Editoval dakur (5. 5. 8:18)

Michwuanquana
Člen | 21
+
0
-

To mě taky napadlo. A nebylo lepší to všechno dát do jedné třídy? Zakládal jsem na tom, aby to šlo volat tzv. fluent interface způsobem. Tedy něco ve stylu:

$this->mailer
    ->create($mailBuilder) // bez parametru by to byl ten BaseMail, nebo by se predaval nazev sablony
    ->to($mail)
    ->build($data)
    ->send();

Editoval Michwuanquana (5. 5. 10:30)

Michwuanquana
Člen | 21
+
0
-

Nakonec jsem to vyřešil takto…

class Mailer
{
	public const TEMPLATE_EXT = '.latte';

	public array $config;
	public TemplateFactory $templateFactory;
	public LinkGenerator $linkGenerator;
	public SmtpSender $sender;
	public Request $request;
	public Message $message;
	public Template $template;
	public string $lang;

	public function __construct(
		array $config,
		TemplateFactory $templateFactory,
		LinkGenerator $linkGenerator,
		SmtpSender $sender,
		Request $request
	)
	{
		$this->config = $config;
		$this->templateFactory = $templateFactory;
		$this->linkGenerator = $linkGenerator;
		$this->sender = $sender;
		$this->request = $request;
	}

	private function createTemplate(string $file, string $lang): Template
	{
		$template = $this->templateFactory->createTemplate();
		$template->setTranslator(new Translator($lang));
		$template->getLatte()->addProvider('uiControl', $this->linkGenerator);
		$template->setFile($this->config['templateDir'] . $file . self::TEMPLATE_EXT);
		return $template;
	}

	private function getLang()
	{
		return preg_match('/^\\/([a-z]{2})\\//', $this->request->getUrl()->path, $matches) ? $matches[1] : Translator::DEFAULT_LANG;
	}

	public function create(string $templateFile, string $lang = null): self
	{
		$this->message = new Message;
		$this->message->setFrom($this->config['fromMail']);
		$this->template = $this->createTemplate($templateFile, $lang ?? $this->getLang());
		Debugger::dump($lang);
		return $this;
	}

	public function to(string|array $recipients): self
	{
		if(!is_array($recipients)) {
			$recipients = [$recipients];
		} else {
			if(!count($recipients))
				throw new InvalidArgumentException("Recipient is an empty array.");
		}

		foreach($recipients as $recipient) {
			if(!Validators::isEmail($recipient))
				throw new InvalidArgumentException ("$recipient is not valid email. All emails was not sent.");
		}

		foreach ($recipients as $recipient) {
			$this->message->addTo($recipient);
		}

		return $this;
	}

	public function body(mixed $data): self
	{
		if(is_array($data)) {
			foreach($data as $key => $value) {
				$this->template->$key = $value;
			}
		} else {
			$this->template->data = $data;
		}

		$this->message->setHtmlBody($this->template->renderToString());
		return $this;
	}

	public function send(bool $dump = false): self
	{
		if($dump)
			$this->dump();
		else
			$this->sender->send($this->message);

		return $this;
	}

	public function dump(): self
	{
		Debugger::dump($this->message);
		return $this;
	}
}

a používám to třeba takto

		$this->mailer
			->create('verification')
			->to('test@gmail.com')
			->body($verifyToken)
			->send();

je s tím, co se týče návrhu kódu, něco špatně, nebo co by bylo lepší řešit jinak? Chci se naučit psát kód pokud možno co nejlépe, nejsrozumitelněji, tak jak by ho psal zkušený programátor.

Editoval Michwuanquana (5. 5. 12:32)

dakur
Člen | 351
+
+3
-

Já bych fluent settery tady nepoužil, protože to umožňuje využít API maileru nevalidním způsobem. Např.:

$this->mailer
	->create('verification')
	->body($verifyToken)
	->send();
// => chybí recipient, ale API mi nezabránilo takový mail poslat

Pokud ti jde o to, aby to bylo čitelné a jasné, co tam předáváš, raději použij named arguments. Např.:

$this->mailer->build(
	template: 'verification',
	to: 'test@gmail.com',
	body: $verifyToken,
)->send();

Efekt je stejný (je to přehledné) a navíc tím vynutíš validní stav – buď předáš všechny povinné argumenty, nebo to selže na typové kontrole.

Editoval dakur (5. 5. 14:35)

petr.pavel
Člen | 531
+
+2
-

Ještě bych podotkl, že mi přijde riskantní hard-codovat do MessageBuilderu pravidlo, jak z url vytahovat jazyk. S tím by měl pracovat pouze router, ten ho předá presenteru a ten ho předá metodě MessageBuilder::create().

Teď ti to funguje, ale časem něco změníš v router a zapomeneš změnit tady, nebo přijde routa, na které to fungovat nebude… Navíc porušení DRY.

Michwuanquana
Člen | 21
+
0
-

dakur

Děkuji za radu, upravím to

petr.pavel napsal(a):

Ještě bych podotkl, že mi přijde riskantní hard-codovat do MessageBuilderu pravidlo, jak z url vytahovat jazyk. S tím by měl pracovat pouze router, ten ho předá presenteru a ten ho předá metodě MessageBuilder::create().

Teď ti to funguje, ale časem něco změníš v router a zapomeneš změnit tady, nebo přijde routa, na které to fungovat nebude… Navíc porušení DRY.

Toho jsem si vědom, ale jak jinak dostat lang do služby Mailer, který se stará jak o sestavení Message, tak o samotné odesílání? Chtěl jsem to mít v jedné komponentě a SMTP údaje nastavit v configu, tak, aby se mail dal odeslat jedním příkazem viz výše. Odesílání se volá v třeba i v modelu (service) UserManager, kde po registraci to pošle verifikační mail. Takže to posílání mailu mám přesunout do presenteru? Vím, že ten hard-codovaný lang je prasárna, kdy to nejde použít třeba při automaticky odesílaných mailech přes cron, bylo to dočasné řešení. Proto je i v metodě create druhý parametr lang.

Edit: Udělal jsem to tak, po volání register, ze které si vrátím entitu User, pak volám v presenteru odesílání mailu.

Editoval Michwuanquana (5. 5. 22:11)

petr.pavel
Člen | 531
+
0
-

Služba UserManager by mohla mít závislost na Nette\Application\Request a z něj dostaneš parametr routy taky.