Issues in InstanceCreator.php (master) - Issues in master - dynamic/silverstripe-salsify - Measure and Improve Code Quality continuously with Scrutinizer

Issues (146)

src/Traits/InstanceCreator.php (2 issues)

Labels

Severity

Minor 2

<?php

namespace Dynamic\Salsify\Traits;

use Dynamic\Salsify\Model\Fetcher;
use Dynamic\Salsify\Model\Mapper;
use SilverStripe\Core\Injector\Injector;
use SilverStripe\Security\Member;
use SilverStripe\Security\Security;

/**
 * Trait InstanceCreator
 */
trait InstanceCreator
{
    /**
     * @var Fetcher
     */
    private $fetcher;

    /**
     * @var Mapper
     */
    private $mapper;

    /**
     * @var Member
     */
    private $previousUser;

    /**
     * @return string
     */
    abstract protected function getImporterKey();

    /**
     * @return string
     */
    protected function getMapperInstanceString()
    {
        return Mapper::class . '.' . $this->getImporterKey();
    }

    /**
     * @return string
     */
    protected function getFetcherInstanceString()
    {
        return Fetcher::class . '.' . $this->getImporterKey();
    }

    /**
     * @return \SilverStripe\ORM\DataObject|Member
     * @throws \SilverStripe\ORM\ValidationException
     */
    protected function findOrCreateSalsifyUser()
    {
        if ($member = Member::get()->filter('Email', 'salsify')->first()) {
            return $member;
        }

        $member = Member::create();
        $member->FirstName = 'Salsify';
        $member->Surname = 'Integration';
        $member->Email = 'salsify';
        $member->write();

        return $member;
    }

    /**
     *
     */
    protected function changeToSalsifyUser()
    {
        $this->previousUser = Security::getCurrentUser();
        return Security::setCurrentUser($this->findOrCreateSalsifyUser());
class A
{
    function getObject()
    {
        return null;
    }

}

$a = new A();
if ($a->getObject()) {
    }

    /**
     *
     */
    protected function changeToPreviousUser()
    {
        Security::setCurrentUser($this->previousUser);
        $this->previousUser = null;
    }

    /**
     * @param $className
     * @return bool
     */
    protected function hasService($className)
    {
        return Injector::inst()->has($className . '.' . $this->getImporterKey());
    }

    /**
     * Creates Injector services if they are not defined
     */
    public function createServices()
    {
        if (!Injector::inst()->has($this->getMapperInstanceString())) {
            Injector::inst()->load([
                $this->getMapperInstanceString() => [
                    'class' => Mapper::class,
                ],
            ]);
        }
        if (!Injector::inst()->has($this->getFetcherInstanceString())) {
            Injector::inst()->load([
                $this->getFetcherInstanceString() => [
                    'class' => Fetcher::class,
                ],
            ]);
        }
    }

    /**
     * @return Fetcher
     * @throws \Exception
     */
    public function getFetcher()
    {
        if (!$this->fetcher) {
            $this->setFetcher();
        }
        return $this->fetcher;
    }

    /**
     * @throws \Exception
     */
    protected function setFetcher()
    {
        $this->fetcher = Injector::inst()->createWithArgs($this->getFetcherInstanceString(), [
            'importerKey' => $this->getImporterKey(),
            'noChannel' => property_exists($this, 'noChannel') ? $this->noChannel : false,
        ]);
    }

    /**
     * @return Mapper
     * @throws \Exception
     */
    public function getMapper()
    {
        if (!$this->mapper) {
            $this->setMapper();
        }
        return $this->mapper;
    }

    /**
     * @throws \Exception
     */
    protected function setMapper()
    {
        $this->mapper = Injector::inst()->createWithArgs($this->getMapperInstanceString(), [
            'importerKey' => $this->getImporterKey(),
            'file' => property_exists($this, 'file') ? $this->file : null,
        ]);

        $configKeys = [
            'apiKey',
            'timeout',
            'organizationID',
        ];
        for ($i = 0; $i < count($configKeys); $i++) {
for ($i=0; $i<count($array); $i++) { // calls count() on each iteration
}

// Better
for ($i=0, $c=count($array); $i<$c; $i++) { // calls count() just once
}
            $currentKey = $configKeys[$i];
            $fetcherConfig = $this->getFetcher()->config()->get($currentKey);
            $this->mapper->config()->set($currentKey, $fetcherConfig);
        }
    }
}


1			<?php
2
3			namespace Dynamic\Salsify\Traits;
4
5			use Dynamic\Salsify\Model\Fetcher;
6			use Dynamic\Salsify\Model\Mapper;
7			use SilverStripe\Core\Injector\Injector;
8			use SilverStripe\Security\Member;
9			use SilverStripe\Security\Security;
10
11			/**
12			* Trait InstanceCreator
13			*/
14			trait InstanceCreator
15			{
16			/**
17			* @var Fetcher
18			*/
19			private $fetcher;
20
21			/**
22			* @var Mapper
23			*/
24			private $mapper;
25
26			/**
27			* @var Member
28			*/
29			private $previousUser;
30
31			/**
32			* @return string
33			*/
34			abstract protected function getImporterKey();
35
36			/**
37			* @return string
38			*/
39			protected function getMapperInstanceString()
40			{
41			return Mapper::class . '.' . $this->getImporterKey();
42			}
43
44			/**
45			* @return string
46			*/
47			protected function getFetcherInstanceString()
48			{
49			return Fetcher::class . '.' . $this->getImporterKey();
50			}
51
52			/**
53			* @return \SilverStripe\ORM\DataObject\|Member
54			* @throws \SilverStripe\ORM\ValidationException
55			*/
56			protected function findOrCreateSalsifyUser()
57			{
58			if ($member = Member::get()->filter('Email', 'salsify')->first()) {
59			return $member;
60			}
61
62			$member = Member::create();
63			$member->FirstName = 'Salsify';
64			$member->Surname = 'Integration';
65			$member->Email = 'salsify';
66			$member->write();
67
68			return $member;
69			}
70
71			/**
72			*
73			*/
74			protected function changeToSalsifyUser()
75			{
76			$this->previousUser = Security::getCurrentUser();
77			return Security::setCurrentUser($this->findOrCreateSalsifyUser());
			0 ignored issues – show Bug introduced 2019-12-12 21:05 UTC by Report Bug Copy Issue Report Show Similar Issues like this Are you sure the usage of `SilverStripe\Security\Se...dOrCreateSalsifyUser())` targeting `SilverStripe\Security\Security::setCurrentUser()` seems to always return null. This check looks for function or method calls that always return null and whose return value is used. class A { function getObject() { return null; } } $a = new A(); if ($a->getObject()) { The method `getObject()` can return nothing but null, so it makes no sense to use the return value. The reason is most likely that a function or method is imcomplete or has been reduced for debug purposes. Loading history...
78			}
79
80			/**
81			*
82			*/
83			protected function changeToPreviousUser()
84			{
85			Security::setCurrentUser($this->previousUser);
86			$this->previousUser = null;
87			}
88
89			/**
90			* @param $className
91			* @return bool
92			*/
93			protected function hasService($className)
94			{
95			return Injector::inst()->has($className . '.' . $this->getImporterKey());
96			}
97
98			/**
99			* Creates Injector services if they are not defined
100			*/
101			public function createServices()
102			{
103			if (!Injector::inst()->has($this->getMapperInstanceString())) {
104			Injector::inst()->load([
105			$this->getMapperInstanceString() => [
106			'class' => Mapper::class,
107			],
108			]);
109			}
110			if (!Injector::inst()->has($this->getFetcherInstanceString())) {
111			Injector::inst()->load([
112			$this->getFetcherInstanceString() => [
113			'class' => Fetcher::class,
114			],
115			]);
116			}
117			}
118
119			/**
120			* @return Fetcher
121			* @throws \Exception
122			*/
123			public function getFetcher()
124			{
125			if (!$this->fetcher) {
126			$this->setFetcher();
127			}
128			return $this->fetcher;
129			}
130
131			/**
132			* @throws \Exception
133			*/
134			protected function setFetcher()
135			{
136			$this->fetcher = Injector::inst()->createWithArgs($this->getFetcherInstanceString(), [
137			'importerKey' => $this->getImporterKey(),
138			'noChannel' => property_exists($this, 'noChannel') ? $this->noChannel : false,
139			]);
140			}
141
142			/**
143			* @return Mapper
144			* @throws \Exception
145			*/
146			public function getMapper()
147			{
148			if (!$this->mapper) {
149			$this->setMapper();
150			}
151			return $this->mapper;
152			}
153
154			/**
155			* @throws \Exception
156			*/
157			protected function setMapper()
158			{
159			$this->mapper = Injector::inst()->createWithArgs($this->getMapperInstanceString(), [
160			'importerKey' => $this->getImporterKey(),
161			'file' => property_exists($this, 'file') ? $this->file : null,
162			]);
163
164			$configKeys = [
165			'apiKey',
166			'timeout',
167			'organizationID',
168			];
169			for ($i = 0; $i < count($configKeys); $i++) {
			0 ignored issues – show Performance Best Practice introduced 2019-08-09 16:11 UTC by Report Bug Copy Issue Report Show Similar Issues like this It seems like you are calling the size function `count()` as part of the test condition. You might want to compute the size beforehand, and not on each iteration. If the size of the collection does not change during the iteration, it is generally a good practice to compute it beforehand, and not on each iteration: for ($i=0; $i<count($array); $i++) { // calls count() on each iteration } // Better for ($i=0, $c=count($array); $i<$c; $i++) { // calls count() just once } Loading history...
170			$currentKey = $configKeys[$i];
171			$fetcherConfig = $this->getFetcher()->config()->get($currentKey);
172			$this->mapper->config()->set($currentKey, $fetcherConfig);
173			}
174			}
175			}
176

dynamic / silverstripe-salsify

Issues (146)

src/Traits/InstanceCreator.php (2 issues)

Labels

Severity

Introduced By

Duplication Side-by-Side

Filter issues like