I just read an article about service subscribers. And I thought to myself why is this still a valid practice?
It is been years that I loaded a dependency in the constructor because it was needed by one or more methods. The AbstractController is the first class I remove when I set up a new Symfony project.
What to do instead
I'm going to take the class from the article as a reference.
final class UserReport extends AbstractReport
{
public function __construct(private readonly CsvGenerator $csvGenerator)
{}
public function generateMail(string $recipientEmail): void
{
/* rest of your code */
$this->getMailer()->send(/* send email */);
$this->getLogger()->info('Email report sent to ' . $recipientEmail);
}
public function generatePdf(): StreamedResponse
{
$pdf = $this->getPdfGenerator();
$this->getLogger()->info('PDF report generated');
return $pdf->html()
->content('content.html.twig')
->generate()
->stream()
;
}
public function generateView(): string
{
$view = $this->getTwig()->render('template.html.twig');
$this->getLogger()->info('Report page generated');
return $view;
}
public function generateCsv(): string
{
$csv = $this->csvGenerator->generate();
/** rest of your code */
}
}
Instead of having everything in one class, use events and listeners to target the dependencies.
class UserReportMailEvent extends Event
{
public function __construct(public UserDTO $user)
{}
}
#[AsEventListener]
class UserReportMailListener
{
public function __construct(
private LoggerInterface $logger,
private MailerInterface $mailer,
)
{}
public function __invoke(UserReportMailEvent $event)
{
/* rest of your code */
$this->mailer->send(/* send email */);
$this->logger->info('Email report sent to ' . $event->user->email);
}
}
In a service you could make it easier to create the event and dispatch it.
enum UserReportAction
{
case Mail;
case Pdf;
case View;
case Csv;
}
class UserReportService
{
public function __construct(private EventDispatcherInterface $eventDispatcher)
{}
public function execute(UserReportAction $action, UserDTO $user)
{
$event = match($action) {
UserReportAction::Mail => new UserReportMailEvent($user),
UserReportAction::Pdf => new UserReportPdfEvent($user),
}
$this->eventDispatcher($event);
}
}
The one thing that is missing is that the most of the actions return a value.
So the code needs a little rewrite.
interface Outputable
{
public mixed $output { get; set; }
}
class UserReportMailEvent extends Event implements Outputable
{
public function __construct(
public UserDTO $user,
public mixed $output = null
)
{}
}
class UserReportService
{
// untouched code removed
public function execute(UserReportAction $action, UserDTO $user)
{
$event = match($action) {
UserReportAction::Mail => new UserReportMailEvent($user),
UserReportAction::Pdf => new UserReportPdfEvent($user),
}
$this->eventDispatcher($event);
// added
return $event->output ?? true;
}
}
And now the output can bubble up.
Conclusion
Think before you use a service subscriber. Most of the times it is possible to prevent occasional dependencies by other means, as I shown in this post.
Another example is controller methods. Instead of loading the dependencies in the constructor, load them as method arguments. This gives you more control, and avoids the use of the AbstractController
.
Top comments (3)
Hey. About the article you mention. It is specifically aimed at reusable bundles to provide a better dx. Anything in the constructor when extending would require to provide the same services / arguments. Sometimes in symfony this isn't easy. Also the article was motivated by a specific need with trait composition where dependencies could vary a lot. See the GotenbergBundle for full understanding of our use cases.
I understand that a service subscriber is a good tool. Like all the good tools they serve a specific purpose. From the article I didn't get that specific purpose, that is why I wrote my post.
It is possible the code is valid in your use case. I just picked the code example that stood out the most for me, to give an example of how I would handle that.
I think blaming the framework is showing your own weakness. A framework provides you with a baseline. You have to bring your programming knowledge to solve problems.
Not blaming the framework. But for example if you extend a class from a service that requires 3 arguments of which two are abstract args... Then you also need a compiler pass to extract that arg from original definition and forward it to your own. I don't call that easy neither am I blaming the framework. But ok. I Get your post.