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

ManageController   A

Complexity

Total Complexity 13

Size/Duplication

Total Lines 114
Duplicated Lines 10.53 %

Coupling/Cohesion

Components 1
Dependencies 8

Importance

Changes 9
Bugs 3 Features 2
Metric Value
wmc 13
c 9
b 3
f 2
lcom 1
cbo 8
dl 12
loc 114
rs 10

2 Methods

Rating   Name   Duplication   Size   Complexity  
A attachDefaultListeners() 0 9 1
C formAction() 12 96 12

How to fix   Duplicated Code   

Duplicated Code

Duplicate code is one of the most pungent code smells. A rule that is often used is to re-structure code once it is duplicated in three or more places.

Common duplication problems, and corresponding solutions are:

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