JobForm::buildForm() - Code Metrics - Inspection of "nieuzywany kod" - adam-boduch/coyote - Measure and Improve Code Quality continuously with Scrutinizer

Completed

Branch — job-vuejs (8df734)

by Adam

created 2017-02-01 16:37 UTC

JobForm::buildForm() B

↳ Parent: JobForm

Complexity

Conditions	1
Paths	1

Size

Total Lines	134
Code Lines	95

Duplication

Lines	0
Ratio	0 %

Importance

Changes

Metric	Value
cc	1
eloc	95
nc	1
nop	0
dl	0
loc	134
rs	8.2857
c	0
b	0
f	0

How to fix Long Method

<?php

namespace Coyote\Http\Forms\Job;

use Coyote\Country;
use Coyote\Currency;
use Coyote\Job;
use Coyote\Services\FormBuilder\Form;
use Coyote\Services\FormBuilder\FormEvents;
use Coyote\Services\Geocoder\GeocoderInterface;
use Coyote\Services\Parser\Helpers\City;
use Coyote\Tag;
use Illuminate\Database\Eloquent\Model;

class JobForm extends Form
{
    /**
     * @var string
     */
    protected $theme = self::THEME_INLINE;

    /**
     * @var Job
     */
    protected $data;

    /**
     * It's public so we can use use attr from twig
     *
     * @var array
     */
    public $attr = [
        'method' => self::POST,
    ];

    /**
     * @var GeocoderInterface
     */
    private $geocoder;

    /**
     * @param GeocoderInterface $geocoder
     */
    public function __construct(GeocoderInterface $geocoder)
    {
        parent::__construct();

        $this->geocoder = $geocoder;

        $this->addEventListener(FormEvents::POST_SUBMIT, function (JobForm $form) {
            $this->forget($this->data->tags);
class User
{
    private $email;

    public function getEmail()
    {
        return $this->email;
    }

    public function setEmail($email)
    {
        $this->email = $email;
    }
}
            $this->forget($this->data->locations);
class User
{
    private $email;

    public function getEmail()
    {
        return $this->email;
    }

    public function setEmail($email)
    {
        $this->email = $email;
    }
}

            // deadline not exists in table "jobs" nor in fillable array. set value so model can transform it
            // to Carbon object
            $this->data->deadline = $form->get('deadline')->getValue();

            foreach ($form->get('tags')->getChildrenValues() as $tag) {
abstract class User
{
    /** @return string */
    abstract public function getPassword();
}

class MyUser extends 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.
    }
}
                $pivot = $this->data->tags()->newPivot(['priority' => $tag['priority']]);
                $model = (new Tag($tag))->setRelation('pivot', $pivot);

                $this->data->tags->add($model);

            }

            $cities = (new City())->grab($form->get('city')->getValue());

            foreach ($cities as $city) {
                $this->data->locations->add(new Job\Location($this->geocode($city)));

            }
        });
    }

    public function buildForm()
    {
        $countryList = Country::pluck('name', 'id')->toArray();

        $this
            ->setAttr(['class' => 'submit-form', 'v-cloak' => 'v-cloak'])
            ->setUrl(route('job.submit'))
            ->add('id', 'hidden')
            ->add('slug', 'hidden')
            ->add('firm_id', 'hidden')
            ->add('title', 'text', [
                'rules' => 'min:2|max:60',
                'label' => 'Tytuł oferty',
                'required' => true,
                'help' => 'Pozostało <strong>${ charCounter(\'job.title\', 60) }</strong> znaków',
                'attr' => [
                    'placeholder' => 'Np. Senior Java Developer',
                    'maxlength' => 60,
                    'v-model' => 'job.title'
                ],
                'row_attr' => [
                    'class' => 'form-group form-group-border'
                ]
            ])
            ->add('country_id', 'select', [
                'rules' => 'required|integer',
                'choices' => $countryList
            ])
            ->add('city', 'text', [
                'rules' => 'string|city',
                'attr' => [
                    'placeholder' => 'Np. Wrocław, Warszawa'
                ]
            ])
            ->add('is_remote', 'checkbox', [
                'label' => 'Możliwa praca zdalna w zakresie',
                'rules' => 'bool',
                'attr' => [
                    'id' => 'remote'
                ],
                'label_attr' => [
                    'for' => 'remote'
                ]

            ])
            ->add('remote_range', 'select', [
                'rules' => 'integer|min:10|max:100',
                'choices' => Job::getRemoteRangeList(),
                'attr' => [
                    'placeholder' => '--',
                    'class' => 'input-sm input-inline',
                    'style' => 'width: 100px'
                ]
            ])
            ->add('salary_from', 'text', [
                'rules' => 'integer',
                'help' => 'Podanie tych informacji nie jest obowiązkowe, ale dzięki temu Twoja oferta zainteresuje więcej osób. Obiecujemy!',
                'attr' => [
                    'class' => 'input-inline'
                ]
            ])
            ->add('salary_to', 'text', [
                'rules' => 'integer',
                'attr' => [
                    'class' => 'input-inline'
                ]
            ])
            ->add('currency_id', 'select', [
                'rules' => 'required|integer',
                'choices' => Currency::pluck('name', 'id')->toArray(),
                'attr' => [
                    'class' => 'input-inline'
                ]
            ])
            ->add('rate_id', 'select', [
                'rules' => 'required|integer',
                'choices' => Job::getRatesList(),
                'attr' => [
                    'class' => 'input-inline'
                ]
            ])
            ->add('employment_id', 'select', [
                'rules' => 'required|integer',
                'choices' => Job::getEmploymentList(),
                'attr' => [
                    'class' => 'input-inline'
                ]
            ])
            ->add('deadline', 'text', [
                'label' => 'Data ważnosci oferty',
                'rules' => 'integer|min:1|max:365',
                'help' => 'Oferta będzie widoczna na stronie do dnia <strong>${ deadlineDate }</strong>',
                'attr' => [
                    'class' => 'input-inline',
                    'v-model' => 'job.deadline'
                ]
            ])
            ->add('tags', 'collection', [
                'child_attr' => [
                    'type' => 'child_form',
                    'class' => TagsForm::class
                ]
            ])
            ->add('description', 'textarea', [
                'label' => 'Opis oferty',
                'help' => 'Miejsce na szczegółowy opis oferty. Pole to jednak nie jest wymagane.',
                'style' => 'height: 140px'
            ])
            ->add('enable_apply', 'choice', [
                'multiple' => false,
                'choices' => [

                    true => 'Zezwól na wysyłanie CV poprzez serwis 4programmers.net',
                    false => '...lub podaj informacje w jaki sposób kandydaci mogą aplikować na to stanowisko',
                ]
            ])
            ->add('recruitment', 'textarea', [
                'rules' => 'required_if:enable_apply,0|string',
                'style' => 'height: 40px'
            ])
            ->add('email', 'email', [
                'rules' => 'sometimes|required|email',
                'help' => 'Podaj adres e-mail na jaki wyślemy Ci informacje o kandydatach. Adres e-mail nie będzie widoczny dla osób postronnych.'
            ])

            ->add('submit', 'submit', [
                'label' => 'Zapisz',
                'attr' => [
                    'data-submit-state' => 'Wysyłanie...'
                ]
            ]);

        $this->setupDefaultValues();
    }

    protected function setupDefaultValues()
    {
        if ($this->data instanceof Model && !$this->data->exists) {
            $this->get('email')->setValue($this->request->user()->email);

            // @todo Uzyc mechanizmu geolokalizacji
            $this->get('country_id')->setValue(array_search('Polska', $this->get('country_id')->getChoices()));
abstract class User
{
    /** @return string */
    abstract public function getPassword();
}

class MyUser extends 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.
    }
}
            $this->get('remote_range')->setValue(100);
        }
    }

    /**
     * @param string $city
     * @return array
     */
    private function geocode($city)
    {
        $location = [
            'city'          => $city
        ];

        try {
            $location = $this->geocoder->geocode($city);

            if (!$location->city) {
                $location->city = $city;
            }

            $location = $location->toArray();
        } catch (\Exception $e) {
            logger()->error($e->getMessage());
        }

        return $location;
    }

    /**
     * @param mixed $collection
     */
    private function forget($collection)
    {
        foreach ($collection as $key => $model) {
            unset($collection[$key]);
        }
    }
}


Branch — job-vuejs (8df734)

JobForm::buildForm() B

Complexity

Size

Duplication

Importance

How to fix Long Method

Long Method

Available Fixes

Available Fixes

1			<?php
2
3			namespace Coyote\Http\Forms\Job;
4
5			use Coyote\Country;
6			use Coyote\Currency;
7			use Coyote\Job;
8			use Coyote\Services\FormBuilder\Form;
9			use Coyote\Services\FormBuilder\FormEvents;
10			use Coyote\Services\Geocoder\GeocoderInterface;
11			use Coyote\Services\Parser\Helpers\City;
12			use Coyote\Tag;
13			use Illuminate\Database\Eloquent\Model;
14
15			class JobForm extends Form
16			{
17			/**
18			* @var string
19			*/
20			protected $theme = self::THEME_INLINE;
21
22			/**
23			* @var Job
24			*/
25			protected $data;
26
27			/**
28			* It's public so we can use use attr from twig
29			*
30			* @var array
31			*/
32			public $attr = [
33			'method' => self::POST,
34			];
35
36			/**
37			* @var GeocoderInterface
38			*/
39			private $geocoder;
40
41			/**
42			* @param GeocoderInterface $geocoder
43			*/
44			public function __construct(GeocoderInterface $geocoder)
45			{
46			parent::__construct();
47
48			$this->geocoder = $geocoder;
49
50			$this->addEventListener(FormEvents::POST_SUBMIT, function (JobForm $form) {
51			$this->forget($this->data->tags);
			1 ignored issue – show Unused Code introduced 2017-02-01 16:54 UTC by Report Bug Copy Issue Report The call to the method `Coyote\Http\Forms\Job\JobForm::forget()` seems un-needed as the method has no side-effects. PHP Analyzer performs a side-effects analysis of your code. A side-effect is basically anything that might be visible after the scope of the method is left. Let’s take a look at an example: class User { private $email; public function getEmail() { return $this->email; } public function setEmail($email) { $this->email = $email; } } If we look at the `getEmail()` method, we can see that it has no side-effect. Whether you call this method or not, no future calls to other methods are affected by this. As such code as the following is useless: $user = new User(); $user->getEmail(); // This line could safely be removed as it has no effect. On the hand, if we look at the `setEmail()`, this method _has_ side-effects. In the following case, we could not remove the method call: $user = new User(); $user->setEmail('email@domain'); // This line has a side-effect (it changes an // instance variable). Loading history...
52			$this->forget($this->data->locations);
			1 ignored issue – show Unused Code introduced 2017-02-01 16:54 UTC by Report Bug Copy Issue Report The call to the method `Coyote\Http\Forms\Job\JobForm::forget()` seems un-needed as the method has no side-effects. PHP Analyzer performs a side-effects analysis of your code. A side-effect is basically anything that might be visible after the scope of the method is left. Let’s take a look at an example: class User { private $email; public function getEmail() { return $this->email; } public function setEmail($email) { $this->email = $email; } } If we look at the `getEmail()` method, we can see that it has no side-effect. Whether you call this method or not, no future calls to other methods are affected by this. As such code as the following is useless: $user = new User(); $user->getEmail(); // This line could safely be removed as it has no effect. On the hand, if we look at the `setEmail()`, this method _has_ side-effects. In the following case, we could not remove the method call: $user = new User(); $user->setEmail('email@domain'); // This line has a side-effect (it changes an // instance variable). Loading history...
53
54			// deadline not exists in table "jobs" nor in fillable array. set value so model can transform it
55			// to Carbon object
56			$this->data->deadline = $form->get('deadline')->getValue();
57
58			foreach ($form->get('tags')->getChildrenValues() as $tag) {
			0 ignored issues – show Bug introduced 2017-02-01 16:54 UTC by Report Bug Copy Issue Report It seems like you code against a specific sub-type and not the parent class `Coyote\Services\FormBuilder\Fields\Field` as the method `getChildrenValues()` does only exist in the following sub-classes of `Coyote\Services\FormBuilder\Fields\Field`: `Coyote\Services\FormBuilder\Fields\Choice`, `Coyote\Services\FormBuilder\Fields\Collection`. Maybe you want to `instanceof` check for one of these explicitly? Let’s take a look at an example: abstract class User { /** @return string / abstract public function getPassword(); } class MyUser extends 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 sub-classes of User which does not have a getDisplayName() method, the code will break. Available Fixes Change the type-hint for the parameter: class AuthSystem { public function authenticate(MyUser $user) { / ... / } } 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. Add the method to the parent class: abstract class User { /** @return string / abstract public function getPassword(); /* @return string */ abstract public function getDisplayName(); } Loading history...
59			$pivot = $this->data->tags()->newPivot(['priority' => $tag['priority']]);
60			$model = (new Tag($tag))->setRelation('pivot', $pivot);
61
62			$this->data->tags->add($model);
			1 ignored issue – show Bug introduced 2017-02-01 16:54 UTC by Report Bug Copy Issue Report The method `add` cannot be called on `$this->data->tags` (of type `array<integer,object<Coyote\Tag>>`). Methods can only be called on objects. This check looks for methods being called on variables that have been inferred to never be objects. Loading history...
63			}
64
65			$cities = (new City())->grab($form->get('city')->getValue());
66
67			foreach ($cities as $city) {
68			$this->data->locations->add(new Job\Location($this->geocode($city)));
			1 ignored issue – show Bug introduced 2017-02-01 16:54 UTC by Report Bug Copy Issue Report The method `add` cannot be called on `$this->data->locations` (of type `array<integer,object<Coyote\Job\Location>>`). Methods can only be called on objects. This check looks for methods being called on variables that have been inferred to never be objects. Loading history...
69			}
70			});
71			}
72
73			public function buildForm()
74			{
75			$countryList = Country::pluck('name', 'id')->toArray();
76
77			$this
78			->setAttr(['class' => 'submit-form', 'v-cloak' => 'v-cloak'])
79			->setUrl(route('job.submit'))
80			->add('id', 'hidden')
81			->add('slug', 'hidden')
82			->add('firm_id', 'hidden')
83			->add('title', 'text', [
84			'rules' => 'min:2\|max:60',
85			'label' => 'Tytuł oferty',
86			'required' => true,
87			'help' => 'Pozostało <strong>${ charCounter(\'job.title\', 60) }</strong> znaków',
88			'attr' => [
89			'placeholder' => 'Np. Senior Java Developer',
90			'maxlength' => 60,
91			'v-model' => 'job.title'
92			],
93			'row_attr' => [
94			'class' => 'form-group form-group-border'
95			]
96			])
97			->add('country_id', 'select', [
98			'rules' => 'required\|integer',
99			'choices' => $countryList
100			])
101			->add('city', 'text', [
102			'rules' => 'string\|city',
103			'attr' => [
104			'placeholder' => 'Np. Wrocław, Warszawa'
105			]
106			])
107			->add('is_remote', 'checkbox', [
108			'label' => 'Możliwa praca zdalna w zakresie',
109			'rules' => 'bool',
110			'attr' => [
111			'id' => 'remote'
112			],
113			'label_attr' => [
114			'for' => 'remote'
115			]
116
117			])
118			->add('remote_range', 'select', [
119			'rules' => 'integer\|min:10\|max:100',
120			'choices' => Job::getRemoteRangeList(),
121			'attr' => [
122			'placeholder' => '--',
123			'class' => 'input-sm input-inline',
124			'style' => 'width: 100px'
125			]
126			])
127			->add('salary_from', 'text', [
128			'rules' => 'integer',
129			'help' => 'Podanie tych informacji nie jest obowiązkowe, ale dzięki temu Twoja oferta zainteresuje więcej osób. Obiecujemy!',
130			'attr' => [
131			'class' => 'input-inline'
132			]
133			])
134			->add('salary_to', 'text', [
135			'rules' => 'integer',
136			'attr' => [
137			'class' => 'input-inline'
138			]
139			])
140			->add('currency_id', 'select', [
141			'rules' => 'required\|integer',
142			'choices' => Currency::pluck('name', 'id')->toArray(),
143			'attr' => [
144			'class' => 'input-inline'
145			]
146			])
147			->add('rate_id', 'select', [
148			'rules' => 'required\|integer',
149			'choices' => Job::getRatesList(),
150			'attr' => [
151			'class' => 'input-inline'
152			]
153			])
154			->add('employment_id', 'select', [
155			'rules' => 'required\|integer',
156			'choices' => Job::getEmploymentList(),
157			'attr' => [
158			'class' => 'input-inline'
159			]
160			])
161			->add('deadline', 'text', [
162			'label' => 'Data ważnosci oferty',
163			'rules' => 'integer\|min:1\|max:365',
164			'help' => 'Oferta będzie widoczna na stronie do dnia <strong>${ deadlineDate }</strong>',
165			'attr' => [
166			'class' => 'input-inline',
167			'v-model' => 'job.deadline'
168			]
169			])
170			->add('tags', 'collection', [
171			'child_attr' => [
172			'type' => 'child_form',
173			'class' => TagsForm::class
174			]
175			])
176			->add('description', 'textarea', [
177			'label' => 'Opis oferty',
178			'help' => 'Miejsce na szczegółowy opis oferty. Pole to jednak nie jest wymagane.',
179			'style' => 'height: 140px'
180			])
181			->add('enable_apply', 'choice', [
182			'multiple' => false,
183			'choices' => [
184
185			true => 'Zezwól na wysyłanie CV poprzez serwis 4programmers.net',
186			false => '...lub podaj informacje w jaki sposób kandydaci mogą aplikować na to stanowisko',
187			]
188			])
189			->add('recruitment', 'textarea', [
190			'rules' => 'required_if:enable_apply,0\|string',
191			'style' => 'height: 40px'
192			])
193			->add('email', 'email', [
194			'rules' => 'sometimes\|required\|email',
195			'help' => 'Podaj adres e-mail na jaki wyślemy Ci informacje o kandydatach. Adres e-mail nie będzie widoczny dla osób postronnych.'
196			])
197
198			->add('submit', 'submit', [
199			'label' => 'Zapisz',
200			'attr' => [
201			'data-submit-state' => 'Wysyłanie...'
202			]
203			]);
204
205			$this->setupDefaultValues();
206			}
207
208			protected function setupDefaultValues()
209			{
210			if ($this->data instanceof Model && !$this->data->exists) {
211			$this->get('email')->setValue($this->request->user()->email);
212
213			// @todo Uzyc mechanizmu geolokalizacji
214			$this->get('country_id')->setValue(array_search('Polska', $this->get('country_id')->getChoices()));
			0 ignored issues – show Bug introduced 2017-02-01 16:54 UTC by Report Bug Copy Issue Report It seems like you code against a specific sub-type and not the parent class `Coyote\Services\FormBuilder\Fields\Field` as the method `getChoices()` does only exist in the following sub-classes of `Coyote\Services\FormBuilder\Fields\Field`: `Coyote\Services\FormBuilder\Fields\Choice`, `Coyote\Services\FormBuilder\Fields\Select`. Maybe you want to `instanceof` check for one of these explicitly? Let’s take a look at an example: abstract class User { /** @return string / abstract public function getPassword(); } class MyUser extends 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 sub-classes of User which does not have a getDisplayName() method, the code will break. Available Fixes Change the type-hint for the parameter: class AuthSystem { public function authenticate(MyUser $user) { / ... / } } 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. Add the method to the parent class: abstract class User { /** @return string / abstract public function getPassword(); /* @return string */ abstract public function getDisplayName(); } Loading history...
215			$this->get('remote_range')->setValue(100);
216			}
217			}
218
219			/**
220			* @param string $city
221			* @return array
222			*/
223			private function geocode($city)
224			{
225			$location = [
226			'city' => $city
227			];
228
229			try {
230			$location = $this->geocoder->geocode($city);
231
232			if (!$location->city) {
233			$location->city = $city;
234			}
235
236			$location = $location->toArray();
237			} catch (\Exception $e) {
238			logger()->error($e->getMessage());
239			}
240
241			return $location;
242			}
243
244			/**
245			* @param mixed $collection
246			*/
247			private function forget($collection)
248			{
249			foreach ($collection as $key => $model) {
250			unset($collection[$key]);
251			}
252			}
253			}
254

adam-boduch / coyote

Branch — job-vuejs (8df734)

JobForm::buildForm() B

Complexity

Size

Duplication

Importance

How to fix Long Method

Long Method

Available Fixes

Available Fixes

Duplication Side-by-Side

Filter issues like