The Fat Controller

In this final video in our Beginners Guide to Symfony 3 tutorial series we cover off a few remaining issues from both code, and usability perspectives.

From a code point of view we are going to look at what happens if we were to split our registerAction into a method which displays the form, and a separate method which handles the form submission. I prefer this approach in the majority of cases as these are, in my opinion, two separate concerns. However, there are issues with doing this for certain form submissions, and Registration is one of those occasions.

Secondly, from a usability point of view, we are going to fix an issue whereby the same username or email address could be submitted, and without some validation rules, the user could be left scratching their head as to why their registration request just failed.

As ever with Symfony (or most modern frameworks) there are numerous ways to approach the following issues and what you are about to see is simply one way. Feel free to do things differently - if you find alternative approaches to be easier then go with what you prefer.

One Controller Action, Or Two Separate Controller Actions?

I'm going to go on record here and say that I don't like how most every PHP framework I have tried pushes you to handle form submissions.

When working with forms, there are two parts to allowing a user to add new data:

• Displaying the form
• Handling the submitted form

My preference is for the way this process is handled in Ruby on Rails, or Elixir's Phoenix Framework. In these frameworks the two processes are separate.

In Symfony (along with many others), these two processes are handled together.

My reasoning for disliking this is the extra complexity it brings. Sure, this is a tough argument to make when working with the typical forms we cover in tutorials. In the real world, the issue is more evident.

A quick bit of code:

    public function registerAction(Request $request)
    {
        $member = new Member();

        $form = $this->createForm(MemberType::class, $member)

        $form->handleRequest($request);

        if ($form->isSubmitted() && $form->isValid()) {

            // often lots of logic here

            // a return statement here

        }

        // another return statement here

It's the if that I don't like. Sure, if / conditional statements are useful, but they increase the mental burden. And as I say, that's not so bad in a tutorial, but in real world projects this becomes more of an issue.

Now, I have heard many counter arguments to this. The usual argument is that there really shouldn't be very much happening in the controller action anyway, and if there is, that's a signal of a different problem.

I agree with this. However, I feel like we set ourselves up for this failure by doing two different tasks in one controller action.

Ok, so this is personal opinion.

To better illustrate my point, let's look at the code for both approaches.

This is the typical way we might see this in Symfony / most PHP frameworks:

<?php

namespace AppBundle\Controller;

use AppBundle\Entity\Member;
use AppBundle\Form\Type\MemberType;
use Sensio\Bundle\FrameworkExtraBundle\Configuration\Method;
use Sensio\Bundle\FrameworkExtraBundle\Configuration\Route;
use Symfony\Bundle\FrameworkBundle\Controller\Controller;
use Symfony\Component\HttpFoundation\Request;
use Symfony\Component\Security\Core\Authentication\Token\UsernamePasswordToken;

class RegistrationController extends Controller
{
    /**
     * @Route("/register", name="registration")
     * @return \Symfony\Component\HttpFoundation\Response
     * @throws \LogicException
     */
    public function registerAction(Request $request)
    {
        $member = new Member();

        $form = $this->createForm(MemberType::class, $member);

        $form->handleRequest($request);

        if ($form->isSubmitted() && $form->isValid()) {

            $password = $this
                ->get('security.password_encoder')
                ->encodePassword(
                    $member,
                    $member->getPlainPassword()
                )
            ;

            $member->setPassword($password);

            $em = $this->getDoctrine()->getManager();

            $em->persist($member);
            $em->flush();

            $token = new UsernamePasswordToken(
                $member,
                $password,
                'main',
                $member->getRoles()
            );

            $this->get('security.token_storage')->setToken($token);
            $this->get('session')->set('_security_main', serialize($token));

            $this->addFlash('success', 'You are now successfully registered!');

            return $this->redirectToRoute('homepage');
        }

        return $this->render('registration/register.html.twig', [
            'registration_form' => $form->createView(),
        ]);
    }
}

And here is an alternative approach:

<?php

namespace AppBundle\Controller;

use AppBundle\Entity\Member;
use AppBundle\Form\Type\MemberType;
use Sensio\Bundle\FrameworkExtraBundle\Configuration\Method;
use Sensio\Bundle\FrameworkExtraBundle\Configuration\Route;
use Symfony\Bundle\FrameworkBundle\Controller\Controller;
use Symfony\Component\HttpFoundation\Request;
use Symfony\Component\Security\Core\Authentication\Token\UsernamePasswordToken;

class RegistrationController extends Controller
{
    /**
     * @Route("/register", name="registration")
     * @return \Symfony\Component\HttpFoundation\Response
     * @throws \LogicException
     */
    public function registerAction(Request $request)
    {
        $member = new Member();

        $form = $this->createMemberRegistrationForm($member);

        return $this->render('registration/register.html.twig', [
            'registration_form' => $form->createView(),
        ]);
    }

    /**
     * @param Request $request
     * @Route("/registration-form-submission", name="handle_registration_form_submission")
     * @Method("POST")
     * @return \Symfony\Component\HttpFoundation\RedirectResponse|\Symfony\Component\HttpFoundation\Response
     * @throws \LogicException
     * @throws \InvalidArgumentException
     */
    public function handleFormSubmissionAction(Request $request)
    {
        $member = new Member();

        $form = $this->createMemberRegistrationForm($member);

        $form->handleRequest($request);

        if ( ! $form->isSubmitted() || ! $form->isValid()) {

            return $this->render('registration/register.html.twig', [
                'registration_form' => $form->createView(),
            ]);
        }

        $password = $this
            ->get('security.password_encoder')
            ->encodePassword(
                $member,
                $member->getPlainPassword()
            )
        ;

        $member->setPassword($password);

        $em = $this->getDoctrine()->getManager();

        $em->persist($member);
        $em->flush();

        $token = new UsernamePasswordToken(
            $member,
            $password,
            'main',
            $member->getRoles()
        );

        $this->get('security.token_storage')->setToken($token);
        $this->get('session')->set('_security_main', serialize($token));

        $this->addFlash('success', 'You are now successfully registered!');

        return $this->redirectToRoute('homepage');
    }

    /**
     * @param $member
     * @return \Symfony\Component\Form\Form
     */
    private function createMemberRegistrationForm($member)
    {
        return $this->createForm(MemberType::class, $member, [
            'action' => $this->generateUrl('handle_registration_form_submission')
        ]);
    }
}

Please wait whilst I don my flame retardant suit.

So, wait, I'm trying to argue that my preferred method is easier to comprehend, yet it has three methods instead of one, 96 lines instead of 63, and what looks like a bunch of duplication?

This hasn't even touched on negative impact on the end user experience.

Good shout Chris :)

Remember though: personal opinion. You are free to disregard not only this, but anything I share with you. If you find a way that makes more sense to you then my advice is to go with it.

But indulge me just a little longer, if you will?

I would rather have more lines of code if the resulting code is easier to understand.

For me, having a method that is concerned with simply displaying the form, and a totally different method for processing the submitted form data makes more sense.

I fight to remove indentation (if / conditionals) where at all possible. I find this dramatically aids debugging, whether using a tool, or trying to juggle the code in your head.

The duplication is unfortunate, and can be somewhat mitigated with the use of the private method for creating the form.

The creation of the encoded $password, along with the UsernamePasswordToken inside handleFormSubmissionAction could be extracted also. If this method grew any further this would be a good idea regardless of which approach you prefer.

This approach also adds in complexity around displaying validation feedback. Please watch the video for a better understanding of what I mean here.

Yet, with all this said, I do prefer it. Sure, it looks like it makes life harder in terms of user registration. There's always an exception to the rule. Generally, I have found this approach makes my life, personally, that little bit easier on larger codebases. Your mileage may vary, and that's ok :)

Unique Usernames and Emails

To finish up with our Registration form work flow we are going to address a problem that we definitely do not want: duplicates.

I've seen some wild mitigation strategies to duplication in my time (real crowd pleasing anecdotes - at the right kind of party, of course), but the easiest way to deal with duplicates is to simply stop them from ever occurring.

Thankfully Symfony, or more accurately Doctrine has our back here.

But as ever, there's a little weirdness to this that has caught me out in the past so let's cover it in a little more detail. First, some code:

<?php

// /src/AppBundle/Entity/Member.php

namespace AppBundle\Entity;

use Doctrine\ORM\Mapping as ORM;
use Symfony\Component\Security\Core\User\UserInterface;
use Symfony\Bridge\Doctrine\Validator\Constraints\UniqueEntity;

/**
 * Member
 *
 * @ORM\Table(name="member")
 * @ORM\Entity(repositoryClass="AppBundle\Repository\MemberRepository")
 * @UniqueEntity("username")
 * @UniqueEntity("email")
 */
class Member implements UserInterface, \Serializable
{
    /**
     * @var int
     *
     * @ORM\Column(name="id", type="integer")
     * @ORM\Id
     * @ORM\GeneratedValue(strategy="AUTO")
     */
    private $id;

    /**
     * @var string
     *
     * @ORM\Column(name="username", type="string", length=255, unique=true)
     */
    private $username;

    /**
     * @var string
     *
     * @ORM\Column(name="email", type="string", length=255, unique=true)
     */
    private $email;

    // * snip *

What's new here? The use of the UniqueEntity constraint.

Note here the inclusion of the use statement also:

use Symfony\Bridge\Doctrine\Validator\Constraints\UniqueEntity;

Now onto the point of potential confusion.

We want to ensure that when a user signs up, the given information is valid. In this case, a valid user should have a unique username, and a unique email address.

Why then, do we have to declare two UniqueEntity annotations above our class?

/**
 * Member
 *
 * @ORM\Table(name="member")
 * @ORM\Entity(repositoryClass="AppBundle\Repository\MemberRepository")
 * @UniqueEntity("username")
 * @UniqueEntity("email")
 */
class Member implements UserInterface, \Serializable

A glance at the official documentation for UniqueEntity reveals there is a fields option. Couldn't we therefore remove the seemingly duplicated UniqueEntity annotation, and replace it with this:

/**
 * Member
 *
 * @ORM\Table(name="member")
 * @ORM\Entity(repositoryClass="AppBundle\Repository\MemberRepository")
 * @UniqueEntity(fields={"username", email"}) - yikes!
 */
class Member implements UserInterface, \Serializable

I will admit to it, this caught me. Read the docs, Chris!

See what could potentially happen here if we use fields in this way is that we are saying to be unique, only the combination of the given fields needs to be unique. This means the following are all valid combos in the same database:

• username: "tim", email: "hello@example.com"
• username: "bob", email: "hello@example.com"
• username: "tim", email: "tim@another.com"

Now, we've been extra careful and as such we have a second layer of protection by way of the unique property on our $username and $email class properties:

    /**
     * @var string
     *
     * @ORM\Column(name="username", type="string", length=255, unique=true)
     */
    private $username;

    /**
     * @var string
     *
     * @ORM\Column(name="email", type="string", length=255, unique=true)
     */
    private $email;

However, that won't help our usability at all. In fact, it would throw a 500 error which is fairly nasty:

An exception occurred while executing 'INSERT INTO member (username, email, password) VALUES (?, ?, ?)' with params ["e", "e@e.com", "$2y$13$T3uo9ICabP\/.Td\/FpoEDF.aT.UAX7bW0FrirkzXbP78KTAgfpYOFO"]:

SQLSTATE[23000]: Integrity constraint violation: 1062 Duplicate entry 'e' for key 'UNIQ_70E4FA78F85E0677'

Anyway, just be careful with this. We do need the two separate UniqueEntity annotations.

Form validation is one of the most useful things inside Symfony - it's a huge time saver. If you'd like to know more, I would recommend the Validating Form Data with Symfony 3 video.

That just about wraps us up for this series. We've covered a lot, and I hope you've found the course enjoyable and educational along the way.

I wanted to end this course with my alternative form implementation for the simple reason that code is a personal thing. After you've followed a number of tutorials you will have to start writing code of your own. How you do this can take guidance from examples such as this, but there is rarely one correct way. Don't be put off by this, rather I hope you fully embrace it. I am aware this point of view is somewhat contentious :)

As ever, thank you very much for watching this video as well as the rest of the course. If you have any question, comments, feedback, or suggestions then please do leave them either in this video's comments, or in the comment's of the video for which the question is most relevant.

Chris