Navigation Menu

Skip to content

Instantly share code, notes, and snippets.

@javiereguiluz
Last active August 27, 2016 17:47
Show Gist options
  • Star 4 You must be signed in to star a gist
  • Fork 1 You must be signed in to fork a gist
  • Save javiereguiluz/a66f084a4f2cf11cf0ee to your computer and use it in GitHub Desktop.
Save javiereguiluz/a66f084a4f2cf11cf0ee to your computer and use it in GitHub Desktop.
Discussion about the different solutions to display a flash message

The problem to solve

We want to show a flash message as the result of executing some controller. This message will only last for the next request.

Proposed Solution #1

I propose to use the new addFlash() method available in the base controller of Symfony 2.6:

use Symfony\Bundle\FrameworkBundle\Controller\Controller;

class MyController extends Controller
{
    public function myAction()
    {
        // ... 3 or 4 lines of code

        $this->addFlash('success', 'The product was successfully inserted.');

        return $this->render(...);
    }
}

Requirements of the solution

  • 0 new custom classes
  • 0 new custom methods or functions (addFlash() is provided by the framework)
  • 0 new custom listeners, services or parameters

Total time required by the solution is about 3 seconds, the time needed to type $this->addFlash()

Drawbacks of the solution

Your controller code is coupled with the base controller provided with Symfony. It won't work outside a Symfony application.

Final comments

I don't mind about the coupling in this case because:

  • Controllers are a thin layer of glue-code provided by the framework. Coupling to it allows me to be more productive.
  • I advocate for agressively decoupling your business logic from the underlying framework. But at the same time, I recommend to agressively couple your controllers to the framework, to take advantage of its features. Otherwise, what is the point of using a framework?
  • Spending a lot of time decupling your controllers from the framework is not worth it. In case you change your framework, it's 100% sure that you'll have to make lots of changes in your code. Updating 4 or 5 lines of controller code will be fast and easy.

Proposed Solution #2

Proposed by Carles Climent in this comment

class MyController
{
    public function myAction()
    {
        // ... 3 or 4 lines of code

        $this->get('flash_handler')->add('success', 'The product was successfully inserted.');

        return $this->container->get('templating')->renderResponse(...);
    }
}

Requirements of the solution

First, you have to define these two classes:

interface FlashHandler
{
    public function add($severity, $message);
}
class SymfonyFlashHandler implements FlashHandler
{
    private $session;

    public function __construct(SessionInterface $session)
    {
        $this->session = $session;
    }

    public function add($severity, $message)
    {
        $this->session->getFlashBag()->add($severity, $message);

        return $this;
    }
}

Then, you have to define a new service for the flash handler:

parameters:
    flash_handler.class: [...]\FlashHandler

services:
    flash_handler:
        class:           "%flash_handler.class%"
        arguments:       [@session]

The main advantage of this solution is that Controller is no longer coupled with Symfony. It will work outside a Symfony application.

Other Proposed Solutions

Please propose other solutions to this problem and don't forget to provide the full code needed by your solution. This way we'll be able to compare them all. Thanks!

@carlescliment
Copy link

Well, coupling is something to have always in mind, in my opinion, even when we are talking about controllers.

I see some problems with the $this->addFlash() solution:

  • what happens if you want to replace (for some reason) the object in charge of adding flash messages? I mean, completely remove the flash system from the framework.
  • how many wrapping methods is it ok to add? If we keep adding helper methods to controllers, the amount of responsibilities will make them unmantainable.
  • The framework is just a tool, and like all the tools, it will get rusty and old some day. The more you couple your app to your framework, the more you are dangerously comprimising it.

So what's the point of adding the new method? Saving a few seconds every time you write an addFlash()? Is it worth it? Really?

If you don't like $this->get('session')->getFlashBag()->add($severity, $message) you can create your own helper (or the framework could give you one) so that you could do something like:

$this->get('flash_helper')->add($severity, $message);

Which is a bit longer than

$this->addFlash($severity, $message);

But hasn't any of the problems exposed before.

@carlescliment
Copy link

Btw, what the hell is wrong with adding classes?

@kirkbushell
Copy link

I'm curious how the "no new methods or functions" requirement was done.

@carlescliment
Copy link

Yep, me too, just realized I ignored the requirements part.

@javiereguiluz
Copy link
Author

Here are my answers for your questions:

what happens if you want to replace (for some reason) the object in charge of adding flash messages?

Then you don't use $this->addFlash() and write your own flash message logic.

how many wrapping methods is it ok to add?

I don't know. But in this example, one new method is enough and that's what we added.

Instead of $this->addFlash(), use $this->get('session')->getFlashBag()->add($severity, $message) or $this->get('flash_helper')->add($severity, $message)

I don't get it. To me, addFlash() is as coupled as get('flash_helper')->add() or get('session')->getFlashBag()->add()

Btw, what the hell is wrong with adding classes?

Nothing at all. I just wanted to point that my proposal requires to create 0 classes, 0 methods, 0 functions, 0 listeners, 0 services and 0 parameters.

@javiereguiluz
Copy link
Author

@kirkbushell @carlescliment I've just updated the solution. I mean no custom method or function is needed because addFlash() is provided by the framework.

@carlescliment
Copy link

Then you don't use $this->addFlash() and write your own flash message logic.

So you would have to rewrite every single call in your app. All that seconds won by typing less would get lost. By having a service and a common interface, you could just replace the service. No changes in controllers.

I don't know. But in this example, one new method is enough and that's what we added.

Have a look at the book The Pragmatic Programmer and the "boiled frogs" metaphore. If you put a frog into boiling water, it will jump out. If you progressively heat the water with the frog inside, it will get boiled without noticing.

Don't be a frog!

@javiereguiluz
Copy link
Author

By having a service and a common interface, you could just replace the service. No changes in controllers.

Thanks to my IDE or text editor, I would change all my controllers in about 15 seconds:

find_and_replace

In any case, I'd love to see the full code of the other solutions. It's the only fair way to compare them all.

@carlescliment
Copy link

You mean the code for the 'flash_helper' service? Seems trivial, isn't it?

The interface:

interface FlashHandler
{
    public function add($severity, $message);
}

The class for Symfony, easily replaceable even without an IDE ;)

class SymfonyFlashHandler implements FlashHandler
{
    private $session;

    public function __construct(SessionInterface $session)
    {
        $this->session = $session;
    }

    public function add($severity, $message)
    {
        $this->session->getFlashBag()->add($severity, $message);

        return $this;
    }
}

@javiereguiluz
Copy link
Author

@carlescliment thanks for providing the code! Unfortunately, that solution is incomplete and cannot be used "as is" in a Symfony application. Please, provide all the service configuration needed for your solution and an example of how to use it in a controller. Thanks!

@carlescliment
Copy link

I thought this was a discussion, not an examination. I think I've written enough details to understand the solution.

You can find an example of how to use it from the controllers in my first post. I'm pretty sure you know how to define a service in Symfony 2.

Cheers.

@javiereguiluz
Copy link
Author

@carlescliment, I've updated the Gist with your solution. I like your proposal, but in my opinion it has three important drawbacks:

  • It's error prone. You are no longer using standard Symfony but your framework. This means that you have to train your team to use the new and custom flash helper instead of the addFlash() method that they know. And probably someone will forget about this sometime.
  • It's not portable. You have to copy+paste these classes and define this service in all your Symfony applications.
  • It's still coupled. The solution uses the Session service provided by Symfony, so it won't work outside Symfony.

@carlescliment
Copy link

This means that you have to train your team to use the new and custom flash helper instead of the addFlash() method that they know

The team just needs to know the interface, not the implementation details.

It's not portable.

Symfony could provide the helper as a built-in service the same way it provides the wrapper method addMessage(). In case of replacing it with a custom service, any company could separate the adapter into a composer package, making it reusable.

It's still coupled. The solution uses the Session service provided by Symfony, so it won't work outside Symfony

No, it isn't. Only the SymfonyFlashHandler would be coupled to Symfony. You could write any other implementation of FlashHandler with different constructor and dependencies.

Anyway, let's say it's a matter of taste then. I prefer more typing and less coupling.

@markitosgv
Copy link

Another interesting solution is using event dispatcher and listener like FOSUserBundle:

In controller:

class ProfileController extends Controller
{
    ...

    /**
     * Edit the user
     */
    public function editAction(Request $request)
    {
        ...

        if ($form->isValid()) {
             ...

            $dispatcher->dispatch(FOSUserEvents::PROFILE_EDIT_COMPLETED, new FilterUserResponseEvent($user, $request, $response));

            return $response;
        }

        ...
    }
}

In Listener (Event Subscriber):

<?php

/*
 * This file is part of the FOSUserBundle package.
 *
 * (c) FriendsOfSymfony <http://friendsofsymfony.github.com/>
 *
 * For the full copyright and license information, please view the LICENSE
 * file that was distributed with this source code.
 */

namespace FOS\UserBundle\EventListener;

use FOS\UserBundle\FOSUserEvents;
use Symfony\Component\EventDispatcher\Event;
use Symfony\Component\EventDispatcher\EventSubscriberInterface;
use Symfony\Component\HttpFoundation\Session\Session;
use Symfony\Component\Translation\TranslatorInterface;

class FlashListener implements EventSubscriberInterface
{
    private static $successMessages = array(
        FOSUserEvents::CHANGE_PASSWORD_COMPLETED => 'change_password.flash.success',
        FOSUserEvents::GROUP_CREATE_COMPLETED => 'group.flash.created',
        FOSUserEvents::GROUP_DELETE_COMPLETED => 'group.flash.deleted',
        FOSUserEvents::GROUP_EDIT_COMPLETED => 'group.flash.updated',
        FOSUserEvents::PROFILE_EDIT_COMPLETED => 'profile.flash.updated',
        FOSUserEvents::REGISTRATION_COMPLETED => 'registration.flash.user_created',
        FOSUserEvents::RESETTING_RESET_COMPLETED => 'resetting.flash.success',
    );

    private $session;
    private $translator;

    public function __construct(Session $session, TranslatorInterface $translator)
    {
        $this->session = $session;
        $this->translator = $translator;
    }

    public static function getSubscribedEvents()
    {
        return array(
            FOSUserEvents::CHANGE_PASSWORD_COMPLETED => 'addSuccessFlash',
            FOSUserEvents::GROUP_CREATE_COMPLETED => 'addSuccessFlash',
            FOSUserEvents::GROUP_DELETE_COMPLETED => 'addSuccessFlash',
            FOSUserEvents::GROUP_EDIT_COMPLETED => 'addSuccessFlash',
            FOSUserEvents::PROFILE_EDIT_COMPLETED => 'addSuccessFlash',
            FOSUserEvents::REGISTRATION_COMPLETED => 'addSuccessFlash',
            FOSUserEvents::RESETTING_RESET_COMPLETED => 'addSuccessFlash',
        );
    }

    public function addSuccessFlash(Event $event)
    {
        if (!isset(self::$successMessages[$event->getName()])) {
            throw new \InvalidArgumentException('This event does not correspond to a known flash message');
        }

        $this->session->getFlashBag()->add('success', $this->trans(self::$successMessages[$event->getName()]));
    }

    private function trans($message, array $params = array())
    {
        return $this->translator->trans($message, $params, 'FOSUserBundle');
    }
}

@jorgelbg
Copy link

In this case I think I'm ok with coupling with the Symfony2 framework in this case, nevertheless for decoupling I would use a custom FlashHandler as described on Solution # 2.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment