Improving the Forms

After my last post about simplifying symfony and especially the forms framework, I started diving into the forms framework to find out how we could improve it. I tried to find ways to improve the usability of this framework without reducing its mighty possibilities. Quite the contrary, I think that the forms can be made even mightier than they are now.

The concept presented in this article series is not implemented in symfony. It has been developed together with a few other people in the community. With these articles, I would like to receive feedback about how you like the concept and how it can be improved.

  1. Improving the Forms
  2. Improving the Forms: Field Groups
  3. Improving the Forms: Layouts and Formatters

The Situation

In my last post I introduced you to the following situation: A form should allow to edit a user's name, email address and password. The name should only be editable by administrators, while the password should be entered twice to make sure it was entered correctly. The maximum length of the name is 15 characters and the password should not be the same as the name.

We came up with the following class:

// lib/form/doctrine/ProfileForm.class.php
class ProfileForm extends sfFormDoctrine
{
protected $user = null;

public function __construct(sfUser $user)
{
  $this->user = $user;
}

public function configure()
{
  $this->setWidgets(array(
    'name'           => new sfWidgetFormInput(array(), array(
      'max_length' => 15
    ),
    'email'          => new sfWidgetFormInput(),
    'password'       => new sfWidgetFormInputPassword(),
    'password_again' => new sfWidgetFormInputPassword(),
  ));
  $this->widgetSchema->setLabel('password_again', 'Password (again)');

  $this->setValidators(array(
    'name'           => new sfValidatorString(array(
      'max_length' => 15
    ), array(
      'max_length' => 'The name must be 15 characters or longer'
    ),
    'email'          => new sfValidatorEmail(array(), array(
      'required' => 'Please enter your email address',
    )),
    'password'       => new sfValidatorString(array(), array(
      'required' => 'Please enter a password',
    )),
    'password_again' => new sfValidatorPass(),
  ));

  // implement custom validation logic
  $this->validatorSchema->setPostValidator(new sfValidatorAnd(array(
    new sfValidatorSchemaCompare('password_again', '==', 'password', array(
      'invalid' => 'The passwords must be equal'
    )),
    new sfValidatorCallback(array(
      'callback' => array($this, 'comparePasswordAndName')
    )),
  )));

  // only administrators may enter the user name
  if (!$this->user->hasCredential('admin'))
  {
    unset($this['name']);
  }
}

public function comparePasswordAndName(sfValidator $validator, array $values)
{
  // if the field "name" is not editable, it does not exist in $values
  if (array_key_exists('name', $values) && $values['name'] == $values['password'])
  {
    $error = new sfValidatorError($validator,
        'The password must not be the same as the name');

    // throw an error schema so the error appears at the field "password"
    throw new sfValidatorErrorSchema($validator, array('password', $error));
  }
}
}

The Issues

The above form definition bears several issues:

  • It is not possible to abstract form fields that combine a widget, a validator and possible other settings. You might want to use a password field in several forms so it should be possible to extract that definition into a separate class.

  • To configure the form, you need to know about the widget schema and the validator schema. You also need to know where to call which method. For instance, you need to call setWidgets() on the form, setLabels() on the widget schema and setPostValidator() on the validator schema.

  • Nearly none of the default validator error messages like "Invalid." or "Required." are useful in production, thus we always have to configure them. Validators should provide sensible defaults for these errors to reduce the amount of configuration.

  • Some of the used classes have complex constructor signatures that are hard to remember. sfValidatorCallback, for instance is always called with a callback. It should be possible to pass the callback directly to its constructor without having to create additional arrays.

  • If the user is not administrator, the field "name" is not available. Because of that we must implement additional checks every time we access the form values to find out whether the name has been set. But, in fact, the name value should always be there. It just is not always editable.

  • Throwing a custom validation error and binding it to a specific field is tedious.

I was searching for ways to improve these issues and will explain them in more detail below.

Form Field Abstraction

It would be very useful if you could abstract form fields in symfony that combine a validator, a widget and possible other settings. Let's just imagine for this post that sfField is a class that allows bundling a widget and a validator.

I don't think that including the "form" in the name is necessary (f.i. sfFormField). The "sf" prefix is reserved to symfony core classes and I can't think of any other context in the core where an sfField class could serve useful. KISS.

To create our custom password field, we would just extend sfField and combine the logic inside.

// lib/form/fields/PasswordField.class.php
class PasswordField extends sfField
{
public function configure()
{
  $this->setWidget(new sfWidgetInputPassword());
  $this->setValidator(new sfValidatorString(array(), array(
    'required' => 'Please enter a password',
  ));
}
}

Great! A neatly decoupled definition of a password field. But how should we add this field to a form? Probably with the most obvious method name there is for this task: addField().

// lib/form/doctrine/ProfileForm.class.php
class ProfileForm
{
public function configure()
{
  $this->addField('password', new PasswordField());
  ...
}
}

Can it be easier?

You may ask yourself whether we can't just bundle the second field for repeating our password as well. We certainly could, but I'll leave the answer for this question to another post.

Widgets and Validators

As said before, we currently need to know a lot about the form's internals to configure it successfully. We need to know about sfWidgetFormSchema, sfValidatorSchema and sfForm and their respective methods. Couldn't we simplify that somewhat?

Let's wait for a moment. In the previous section we introduced a new class sfField that bundles a widget, a validator and possible other settings. We also introduced a method addField() that adds a form field to the form.

Now we could return the added field from addField() and allow a modification of its widget, validator, label and other properties in a fluent coding style!

Let's look at how the form's configure() method looks like with these modifications:

public function configure()
{
$this->addField('name', new sfField())
     ->setWidget(new sfWidgetInput(array(), array('max_length' => 15)))
     ->setValidator(new sfValidatorString(array('max_length' => 15)));

$this->addField('email', new sfField())
     ->setWidget(new sfWidgetInput())
     ->setValidator(new sfValidatorEmail());

$this->addField('password', new PasswordField());

$this->addField('password_again', new sfField())
     ->setWidget(new sfWidgetInputPassword())
     ->setValidator(new sfValidatorPass())
     ->setLabel('Password (again)');
...
}

Doesn't look too bad. To reduce the amount of code, addField() could always add an instance of sfField, if no field is given, and preconfigure it with sfWidgetInput and sfValidatorString. Then we could introduce new methods sfField::setWidgetOptions(), sfField::setWidgetAttributes() and sfField::setValidatorOptions() that allow a fluent modification of these properties.

public function configure()
{
$this->addField('name') // implicitly sfWidgetInput and sfValidatorString
     ->setWidgetAttributes(array('max_length' => 15))
     ->setValidatorOptions(array('max_length' => 15));

$this->addField('email') // implicitly sfWidgetInput
     ->setValidator(new sfValidatorEmail(array(), array(
       'required' => 'Please enter your email address',
     )));

$this->addField('password', new PasswordField());

$this->addField('password_again')
     ->setWidget(new sfWidgetInputPassword())
     ->setValidator(new sfValidatorPass())
     ->setLabel('Password (again)');
...
}

Again our code became a bit easier to read and write. For reference, here is how the configuration of these settings was like before:

// before
public function configure()
{
$this->setWidgets(array(
  'name'           => new sfWidgetFormInput(array(), array(
    'max_length' => 15
  ),
  'email'          => new sfWidgetFormInput(),
  'password'       => new sfWidgetFormInputPassword(),
  'password_again' => new sfWidgetFormInputPassword(),
));
$this->widgetSchema->setLabel('password_again', 'Password (again)');

$this->setValidators(array(
  'name'           => new sfValidatorString(array(
    'max_length' => 15
  ), array(
    'max_length' => 'The name must be 15 characters or longer'
  ),
  'email'          => new sfValidatorEmail(array(), array(
    'required' => 'Please enter your email address',
  )),
  'password'       => new sfValidatorString(array(), array(
    'required' => 'Please enter a password',
  )),
  'password_again' => new sfValidatorPass(),
));
...
}

Sensible Default Error Messages

Nearly all default error messages of the validators are absolutely unusable in a production environment. If you listen to the big usability gurus like Jakob Nielsen (see his Error Messages Guidelines), error messages should give "Constructive advice on how to fix the problem". "Required." and "Invalid.", unfortunately, do not.

I think that all validators should be preconfigured with sensible error messages. Examples for the "required" and "invalid" errors of sfValidatorEmail are "Please enter an email address" and "The given email address is not valid".

With sensible default messages, we can yet again reduce the complexity of our code:

public function configure()
{
...
$this->addField('email')
     ->setValidator(new sfValidatorEmail());
...
}

Compared to before:

public function configure()
{
...
$this->addField('email')
     ->setValidator(new sfValidatorEmail(array(), array(
       'required' => 'Please enter your email address',
     )));
...
}

The Post Validator

Now I'll look at how to improve the post validator of our profile form. Basically I think the name is a bit weird. I am familiar with the latin expression "post", but wouldn't the english expression "final" fit in better?

Let's imagine we live in an ideal world where setPostValidator() can be moved to sfForm and renamed to setFinalValidator().

$this->setFinalValidator(...);

Okay. Next step: sfValidatorAnd and its contained validators. First of all, I think we could extend sfValidatorAnd to accept an infinite number of arguments or an array containing these arguments. In Java we would achieve this by overloading the method. In PHP we cannot overload methods, but due to the weak typing we can still simulate this behaviour with one method.

There's another candidate for simplification: sfValidatorCallback. Let's just recall the current usage of this class:

new sfValidatorCallback(array('callback' => array($this, 'comparePasswordAndName')));

Why is this so complicated? It's obvious that we want to pass a callback to this class. A callback can be either a single string or an array with two entries. We can again "overload" the constructor to accept either a single argument (a function name) or two arguments (a class or object method).

new sfValidatorCallback($this, 'comparePasswordAndName');

Much better. Last step: sfValidatorSchemaCompare. I think the usage of its constructor is pretty straight-forward. Because I can never remember its name, this validator should be renamed to sfValidatorCompare (KISS). Furthermore it is always necessary to pass an error message to this class, so why not make it the fourth argument?

new sfValidatorCompare('password_repeat', '==', 'password', 'The passwords must match');

Now we can draw the whole picture of our final validator:

$this->setFinalValidator(new sfValidatorAnd(
new sfValidatorCompare('password_repeat', '==', 'password',
    'The passwords must match'),
new sfValidatorCallback($this, 'comparePasswordAndName')
));

Isn't this much nicer to read? Here's the old definition again:

// before
$this->validatorSchema->setPostValidator(new sfValidatorAnd(array(
new sfValidatorSchemaCompare('password_again', '==', 'password', array(
  'invalid' => 'The passwords must be equal'
)),
new sfValidatorCallback(array(
  'callback' => array($this, 'comparePasswordAndName')
)),
)));

Editable vs. Non-Editable

In forms it is very common to display some fields as editable and some fields as non-editable depending on the user's rights. Just think of a common article form. The administrator may be able to change the author of an article, while the editor may only be allowed to see the author.

In the past, I have read several times that people would like to set fields to read-only in the admin generator. One such situation was the discussion about the admin generator for symfony 1.2. Since the admin generator is now based on the forms framework, this would be essentially the same feature.

The idea is to extend the current sfWidgetForm class to have two different states: "editable" and "non-editable". Dependent on the state, each widget renders different HTML. Let's look at sfWidgetFormInputDate for example. If this widget is editable, it displays the three select fields that we all know. But if the widget is not editable, it renders the date as plain text in the configured culture.

Do you notice something? If such a widget is implemented, it is essentially not a form widget anymore, but just "a widget". In this case we could remove the "Form" from its name and reuse the widget in other components, such as sfGrid ;-).

Let's look at the configuration of our "name" field with this new widget concept. Instead of removing the field when the user is not an administrator, we just disable it.

if (!$this->user->hasCredential('admin'))
{
$this->getField('name')->setEditable(false);
}

That's all! Compared to before:

// before
if (!$this->user->hasCredential('admin'))
{
unset($this['name']);
}

The form should still populate the "name" value after submitting so that we do not have to care about whether the field was editable or not. The submitted value should just be ignored then.

Throwing Custom Validation Errors

Let's be honest. Throwing custom validation errors is a pain in the ass. You must remember to pass the validator itself to the error, and then you need to construct an sfValidatorErrorSchema just to associate the error with a field.

In my opinion this should be much easier. The only reason why sfValidatorError requires a passed validator is to populate the error message internally. Why don't we just pass the error message?

Second, sfValidatorError's constructor could simply accept an optional parameter that specifies the field the validator belongs to.

How does that all look like? Let's look at our refactored method comparePasswordAndName():

public function comparePasswordAndName(sfValidator $validator, array $values)
{
  if ($values['name'] == $values['password'])
  {
    throw new sfValidatorError('The password must not be the same as the name',
        'password');
  }
}

Now that was easy! Here's the old definition:

public function comparePasswordAndName(sfValidator $validator, array $values)
{
  // if the field "name" is not editable, it does not exist in $values
  if (array_key_exists('name', $values) && $values['name'] == $values['password'])
  {
    $error = new sfValidatorError($validator,
        'The password must not be the same as the name');

    // throw an error schema so the error appears at the field "password"
    throw new sfValidatorErrorSchema($validator, array('password', $error));
  }
}

Conclusion

I think we don't need to drop object oriented concepts to facilitate the use of the forms. The interface of this framework can be easy while remaining completely well-designed and flexible.

Here you can see the complete new form definition. Especially note the shorter widget names.

// lib/form/doctrine/ProfileForm.class.php
class ProfileForm extends sfFormDoctrine
{
  protected $user = null;

  public function __construct(sfUser $user)
  {
    $this->user = $user;
  }

  public function configure()
  {
    $this->addField('name')
         ->setWidgetAttributes(array('max_length' => 15))
         ->setValidatorOptions(array('max_length' => 15));

    $this->addField('email')
         ->setValidator(new sfValidatorEmail());

    $this->addField('password', new PasswordField());

    $this->addField('password_again')
         ->setWidget(new sfWidgetPassword())
         ->setValidator(new sfValidatorPass())
         ->setLabel('Password (again)');

    $this->setFinalValidator(new sfValidatorAnd(
      new sfValidatorCompare('password_again', '==', 'password',
          'The passwords must be equal'),
      new sfValidatorCallback($this, 'comparePasswordAndName')
    ));

    if (!$this->user->hasCredential('admin'))
    {
      $this->getField('name')->setEditable(false);
    }
  }

  public function comparePasswordAndName(sfValidator $validator, array $values)
  {
    if ($values['name'] == $values['password'])
    {
      throw new sfValidatorError('The password must not be the same as the name',
          'password');
    }
  }
}

The above ideas are only very rough and conceptual. I was thinking about how the interface would be ideal from the user's point of view without verifying whether it is implementable and, if yes, how. But that's a whole different story anyway. Usually, usability should drive implementation, not the other way round.

I did focus this post on this specific ProfileForm. There are several issues with the form framework that remain to be discussed. One example is how to bundle the two password fields and their post validator to be able to just drop them into any form. Another example is the internal processing of embedded forms. But these topics belong entirely to a new post.

How do you like this interface? Has it flaws and how can it be improved?

Discussion