Completed
Pull Request — develop (#235)
by ANTHONIUS
08:47
created

ManageController::formAction()   C

Complexity

Conditions 12
Paths 28

Size

Total Lines 96
Code Lines 58

Duplication

Lines 12
Ratio 12.5 %

Importance

Changes 7
Bugs 1 Features 2
Metric Value
c 7
b 1
f 2
dl 12
loc 96
rs 5.034
cc 12
eloc 58
nc 28
nop 0

How to fix   Long Method    Complexity   

Long Method

Small methods make your code easier to understand, in particular if combined with a good name. Besides, if your method is small, finding a good name is usually much easier.

For example, if you find yourself adding comments to a method's body, this is usually a good sign to extract the commented part to a new method, and use the comment as a starting point when coming up with a good name for this new method.

Commonly applied refactorings include:

1
<?php
2
/**
3
 * YAWIK
4
 *
5
 * @filesource
6
 * @copyright (c) 2013 - 2016 Cross Solution (http://cross-solution.de)
7
 * @license   MIT
8
 */
9
10
/** ActionController of Core */
11
namespace Cv\Controller;
12
13
use Geo\Form\GeoText;
14
use Zend\Mvc\Controller\AbstractActionController;
15
use Zend\View\Model\JsonModel;
16
use Core\Form\SummaryFormInterface;
17
18
/**
19
 * Main Action Controller for the application.
20
 * Responsible for displaying the home site.
21
 *
22
 */
23
class ManageController extends AbstractActionController
24
{
25
26
    /**
27
     * attaches further Listeners for generating / processing the output
28
     * @return $this
29
     */
30
    public function attachDefaultListeners()
31
    {
32
        parent::attachDefaultListeners();
33
        $serviceLocator  = $this->serviceLocator;
34
        $defaultServices = $serviceLocator->get('DefaultListeners');
35
        $events          = $this->getEventManager();
36
        $events->attach($defaultServices);
0 ignored issues
show
Documentation introduced by
$defaultServices is of type object|array, but the function expects a string.

It seems like the type of the argument is not accepted by the function/method which you are calling.

In some cases, in particular if PHP’s automatic type-juggling kicks in this might be fine. In other cases, however this might be a bug.

We suggest to add an explicit type cast like in the following example:

function acceptsInteger($int) { }

$x = '123'; // string "123"

// Instead of
acceptsInteger($x);

// we recommend to use
acceptsInteger((integer) $x);
Loading history...
37
        return $this;
38
    }
39
    
40
    public function formAction()
41
    {
42
        $serviceLocator = $this->serviceLocator;
43
        $repositories = $serviceLocator->get('repositories');
44
        /* @var $cvRepository \Cv\Repository\Cv */
45
        $cvRepository = $repositories->get('Cv/Cv');
46
        $user = $this->auth()->getUser();
0 ignored issues
show
Documentation Bug introduced by
The method auth does not exist on object<Cv\Controller\ManageController>? Since you implemented __call, maybe consider adding a @method annotation.

If you implement __call and you know which methods are available, you can improve IDE auto-completion and static analysis by adding a @method annotation to the class.

This is often the case, when __call is implemented by a parent class and only the child class knows which methods exist:

class ParentClass {
    private $data = array();

    public function __call($method, array $args) {
        if (0 === strpos($method, 'get')) {
            return $this->data[strtolower(substr($method, 3))];
        }

        throw new \LogicException(sprintf('Unsupported method: %s', $method));
    }
}

/**
 * If this class knows which fields exist, you can specify the methods here:
 *
 * @method string getName()
 */
class SomeClass extends ParentClass { }
Loading history...
47
        /* @var $cv \Cv\Entity\Cv */
48
        $cv = $cvRepository->findDraft($user);
49
        
50
        if (empty($cv)) {
51
            // create draft CV
52
            $cv = $cvRepository->create();
53
            $cv->setIsDraft(true);
54
            $cv->setContact($user->getInfo());
55
            $cv->setUser($user);
56
            $repositories->store($cv);
57
        }
58
        
59
        /* @var $container \Core\Form\Container */
60
        $container = $serviceLocator->get('FormElementManager')
61
            ->get('CvContainer')
62
            ->setEntity($cv);
63
64
        // process post method
65
        if ($this->getRequest()->isPost()) {
0 ignored issues
show
Bug introduced by
It seems like you code against a concrete implementation and not the interface Zend\Stdlib\RequestInterface as the method isPost() does only exist in the following implementations of said interface: Zend\Http\PhpEnvironment\Request, Zend\Http\Request.

Let’s take a look at an example:

interface User
{
    /** @return string */
    public function getPassword();
}

class MyUser implements User
{
    public function getPassword()
    {
        // return something
    }

    public function getDisplayName()
    {
        // return some name.
    }
}

class AuthSystem
{
    public function authenticate(User $user)
    {
        $this->logger->info(sprintf('Authenticating %s.', $user->getDisplayName()));
        // do something.
    }
}

In the above example, the authenticate() method works fine as long as you just pass instances of MyUser. However, if you now also want to pass a different implementation of User which does not have a getDisplayName() method, the code will break.

Available Fixes

  1. Change the type-hint for the parameter:

    class AuthSystem
    {
        public function authenticate(MyUser $user) { /* ... */ }
    }
    
  2. Add an additional type-check:

    class AuthSystem
    {
        public function authenticate(User $user)
        {
            if ($user instanceof MyUser) {
                $this->logger->info(/** ... */);
            }
    
            // or alternatively
            if ( ! $user instanceof MyUser) {
                throw new \LogicException(
                    '$user must be an instance of MyUser, '
                   .'other instances are not supported.'
                );
            }
    
        }
    }
    
Note: PHP Analyzer uses reverse abstract interpretation to narrow down the types inside the if block in such a case.
  1. Add the method to the interface:

    interface User
    {
        /** @return string */
        public function getPassword();
    
        /** @return string */
        public function getDisplayName();
    }
    
Loading history...
66
            $params = $this->params();
67
            $form = $container->getForm($params->fromQuery('form'));
68
69
            if ($form) {
70
                $form->setData(array_merge(
71
                    $params->fromPost(),
72
                    $params->fromFiles()
73
                ));
74
                
75
                if (!$form->isValid()) {
76
                    return new JsonModel([
77
                        'valid' => false,
78
                        'errors' => $form->getMessages()
79
                    ]);
80
                }
81
                /*
82
                 * @todo This is a workaround for GeoJSON data insertion
83
                 * until we figured out, what we really want it to be.
84
                 */
85
                $formId = $params->fromQuery('form');
86 View Code Duplication
                if ('preferredJob' == $formId) {
0 ignored issues
show
Duplication introduced by
This code seems to be duplicated across your project.

Duplicated code is one of the most pungent code smells. If you need to duplicate the same code in three or more different places, we strongly encourage you to look into extracting the code into a single class or operation.

You can also find more detailed suggestions in the “Code” section of your repository.

Loading history...
87
                    $locElem = $form->getBaseFieldset()->get('geo-location');
88
                    if ($locElem instanceof GeoText) {
89
                        $loc = $locElem->getValue('entity');
90
                        $locations = $cv->getPreferredJob()->getDesiredLocations();
91
                        if (count($locations)) {
92
                            $locations->clear();
93
                        }
94
                        $locations->add($loc);
95
                        $cv->getPreferredJob()->setDesiredLocation($locElem->getValue());
96
                    }
97
                }
98
99
                $repositories->store($cv);
100
                
101
                if ($form instanceof SummaryFormInterface) {
102
                    $form->setRenderMode(SummaryFormInterface::RENDER_SUMMARY);
103
                    $viewHelper = 'summaryform';
104
                } else {
105
                    $viewHelper = 'form';
106
                }
107
                
108
                // render form
109
                $content = $serviceLocator->get('ViewHelperManager')
110
                    ->get($viewHelper)
111
                    ->__invoke($form);
112
                
113
                return new JsonModel([
114
                    'valid' => true,
115
                    'content' => $content
116
                ]);
117
            } elseif (($action = $params->fromQuery('action')) !== null) {
118
                return new JsonModel($container->executeAction($action, $params->fromPost()));
119
            }
120
        }// end of process post method
121
        else {
122
            $locElem = $container->getForm('preferredJob')->getBaseFieldset()->get('geo-location');
123
            if ($locElem instanceof GeoText) {
124
                $loc = $cv->getPreferredJob()->getDesiredLocations();
125
                if (count($loc)) {
126
                    $locElem->setValue($loc->first());
127
                }
128
            }
129
        }
130
131
132
        return [
133
            'container' => $container
134
        ];
135
    }
136
}
137