Michal Vyšinský bio photo

Michal Vyšinský

Enthusiastic developer.
HC Kometa Brno fan


Twitter LinkedIn Github

Contents

In each web project you will get into a situation where you need a form for creating and a form for editing items. These forms are almost identical, they have the same fields, the same validation rules but they are processed in a different way. Also, the edit form must be filled with data from database.

There are two common approaches to this situation. I will talk about them and give you a reason why you should not use them. In the end I will give you the best way to handle such situations.

Please note: I am going to use classic Nette Framework's approach for using injectable components. Unfortunately I have not found any English article about generated factories. Long story short: you create a factory interface, register it as a service into the DI container and Nette will generate an implementation for you. I will write an article about it soon.

One class to rule them all…

The first solution that comes to mind. Let’s have one form for create and edit tasks:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
<?php

use Nette\Application\UI\Form;

class UserForm extends Form 
{

    /** @var Model\UserRepository */
    private $userRepository;
    
    /** @var Model\User */
    private $user;

    public function __construct(Model\User $user = NULL, Model\UserRepository $userRepository)
    {
        $this->user = $user;
        $this->userRepository = $userRepository;
    }

    public function attached($presenter) 
    {
        parent::attached($presenter);

        // build form here
        
        if($this->user) {
           //fill form
        }
        $this->onSuccess[] = $this->processForm;
    }    
     
    public function processForm($form, $values)
    {
        if($this->user) {
            $this->userRepository->edit($this->user, $values);
        } else {
            $this->userRepository->create($values);
        }
    }

}

The factory interface for this class could look like this:

1
2
3
4
5
6
7
8
<?php

interface UserFormFactory {

    /** @return UserForm */
    function create(Model\User $user = NULL);
    
}

As one argument we have to pass manually to the factory is $user. $userRepository is an autowired service registered in the DI container.

The factory definition in the configuration neon file will look like this:

1
2
3
4
services:
    - implement: UserFormFactory
      parameters: [Model\User user = NULL]
      arguments: [%user%]

This solution is really bad. Whenever is it possible, it is better to use polymorphism instead of conditions. Also, this class violates SRP. It creates the form, fills the form and also, either creates or edits a user. Ok, let’s go to the second way.

Inheritance - let’s split it

With inheritance we will create some ‘base’ form, which will build the form, one form to create a user and another to edit a user. Sounds as good solution, right? Well, we can try it and to make it more complex - we want to load roles into a selectbox.

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
<?php

use Nette\Application\UI\Form;

abstract class UserForm extends Form 
{

    /** @var Model\RoleRepository */
    private $roleRepository;
    
    public function __construct(Model\RoleRepository $roleRepository)
    {
        $this->roleRepository = $roleRepository;
    }

    public function attached($presenter) 
    {
        parent::attached($presenter);

        // build form here
        $roles = $this->rolesRepository->getPairs('id', 'name');
        $this->addSelect('role', 'Role', $roles);
      
        $this->onSuccess[] = $this->processForm;
    }    
     
    abstract function processForm($form, $values);

}

class UserCreateForm extends UserForm
{

    /** @var Model\UserRepository */
    private $userRepository;        
    
    public function __construct(Model\RoleRepository $roleRepository, Model\UserRepository $userRepository)
    {
        parent::__construct($roleRepository);
        $this->userRepository = $userRepository;
    }
    
    public function processForm($form, $values)
    {
        $this->userRepository->create($values);
    }

}

class UserEditForm extends UserForm
{

    /** @var Model\UserRepository */
    private $userRepository;        
    
    public function __construct(Model\User $user, Model\RoleRepository $roleRepository, Model\UserRepository $userRepository)
    {
        parent::__construct($roleRepository);
        $this->userRepository = $userRepository;
    }
    
    public function attached($presenter)
    {
        parent::attached($presenter);
        // fill form with user's data
    }
    
    public function processForm($form, $values)
    {
        $this->userRepository->edit($values);
    }

}

Now we have interfaces and two neon definitions (I will skip interfaces, it will be as same as previous. Only name, create method’s arguments and @return annotation will be different):

1
2
3
4
5
services:
    - implement: UserCreateFormFactory
    - implement: UserEditFormFactory
      parameters: [Model\User user]
      arguments: [%user%]

Well, we have now separated logic. The code is cleaner. Why is this solution bad too? I will give you a few reasons:

  1. All classes depend on the service Model\RolesRepository. But it is actually used only in the UserForm.
  2. We have to call parent::__construct() and pass all parent’s dependencies
  3. If we need to add for example cities selectbox we need to load a list of cities from the Model\CityRepository. We load it in the UserForm so we add a Model\CityRepository dependency into the constructor. Do you see it? Because of changing one class we had to edit all child classes and also a neon configuration. Ouch… no thanks.

Composition

The best way to solve this simple task is to create three different classes. All the classes will have own neon definitions and there will be two factory interfaces. Well, it looks like more writing. To be honest, it is:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
<?php

use Nette\Application\UI\Form;

class UserFormFactory 
{

    /** @var Model\RoleRepository */
    private $roleRepository;

    public function __construct(Model\RoleRepository $roleRepository)
    {
        $this->roleRepository = $roleRepository;
    }  
    
    public function createForm()
    {
        $form = new Form;
        // build form, load roles into selectbox...        
        return $form;
    }

}

use Nette\Application\UI\Control;

class UserCreateForm extends Control 
{
    
    public $onSuccess = [];

    /** @var UserFormFactory */
    private $userFormFactory;  
    
    /** @var Model\UserRepository */
    private $userRepository;
    
    public function __construct(UserFormFactory $userFormFactory, Model\UserRepository $userRepository)
    {
        $this->userFormFactory = $userFormFactory;
        $this->userRepository = $userRepository;
    }  
    

    protected function createComponentForm()
    {
        $form = $this->userFormFactory->createForm();
        $form->onSuccess[] = $this->processForm;
        return $form;
    }
    
    public function processForm($form, $values)
    {
        $user = $this->userRepository->create($values);
        $this->onSuccess($user);
    }

}

class UserEditForm extends Control 
{
    
    public $onSuccess = [];
    
    /** @var Model\User */
    private $user;

    /** @var UserFormFactory */
    private $userFormFactory;  
    
    /** @var Model\UserRepository */
    private $userRepository;
    
    public function __construct(Model\User $user, UserFormFactory $userFormFactory, Model\UserRepository $userRepository)
    {
        $this->user = $user;
        $this->userFormFactory = $userFormFactory;
        $this->userRepository = $userRepository;
    }  
    

    protected function createComponentForm()
    {
        $form = $this->userFormFactory->createForm();
        // fill form with user's data - call $form->setDefaults()
        $form->onSuccess[] = $this->processForm;
        return $form;
    }
    
    public function processForm($form, $values)
    {
        $this->userRepository->edit($this->user, $values);
        $this->onSuccess($this->user);
    }

}

Factory interfaces:

1
2
3
4
5
6
7
8
9
10
11
12
13
<?php

interface UserCreateFormFactory 
{
    /** @return UserCreateForm */
    function create();
}

interface UserEditFormFactory
{
    /** @return UserEditForm */
    function create(Model\User $user);
}

Neon definitions:

1
2
3
4
5
6
services:
    - UserFormFactory
    - implement: UserCreateFormFactory
    - implement: UserEditFormFactory
      parameters: [Model\User user]     
      arguments: [%user%]

A lot of stuff must have been written. Do you ask, why is it the best solution? Here are my arguments:

  1. Separated dependencies for each class
  2. Editing one class does not affect other classes
  3. Cleaner code
  4. SRP is not violated