MarathonStoreJobBusinessCase::isDependencyCircular() - Code Metrics - Inspection of "code style fixes from scrutinizer-ci detection" - trivago/chapi - Measure and Improve Code Quality continuously with Scrutinizer

Completed

Push — master ( a6590b...26125c )

by Marc

created 2017-03-02 11:07 UTC

isDependencyCircular() C

↳ Parent: MarathonStoreJobBusinessCase

Complexity

Conditions	8
Paths	8

Size

Total Lines	63
Code Lines	19

Duplication

Lines	0
Ratio	0 %

Code Coverage

Tests	18
CRAP Score	8.0093

Importance

Changes

Metric	Value
dl	0
loc	63
ccs	18
cts	19
cp	0.9474
rs	6.8825
c	0
b	0
f	0
cc	8
eloc	19
nc	8
nop	3
crap	8.0093

How to fix Long Method

<?php
/**
 *
 * @package: chapi
 *
 * @author: bthapaliya
 * @since: 2017-01-03
 *
 */

namespace Chapi\BusinessCase\JobManagement;


use Chapi\BusinessCase\Comparison\JobComparisonInterface;
use Chapi\Entity\Marathon\MarathonAppEntity;
use Chapi\Service\JobIndex\JobIndexServiceInterface;
use Chapi\Service\JobRepository\JobRepositoryInterface;
use Psr\Log\LoggerInterface;

class MarathonStoreJobBusinessCase extends AbstractStoreJobBusinessCase implements StoreJobBusinessCaseInterface
{
    /**
     * MarathonStoreJobBusinessCase constructor.
     * @param JobIndexServiceInterface $oJobIndexService
     * @param JobRepositoryInterface $oJobRepositoryRemote
     * @param JobRepositoryInterface $oJobRepositoryLocal
     * @param JobComparisonInterface $oJobComparisonBusinessCase
     * @param LoggerInterface $oLogger
     */
    public function __construct(

        JobIndexServiceInterface $oJobIndexService,
        JobRepositoryInterface $oJobRepositoryRemote,
        JobRepositoryInterface $oJobRepositoryLocal,
        JobComparisonInterface $oJobComparisonBusinessCase,
        LoggerInterface $oLogger

    )
    {

        $this->oJobIndexService = $oJobIndexService;
$a = "a";
$ab = "ab";
$abc = "abc";
        $this->oLogger = $oLogger;
$a = "a";
$ab = "ab";
$abc = "abc";
        $this->oJobComparisonBusinessCase = $oJobComparisonBusinessCase;
        $this->oJobRepositoryRemote = $oJobRepositoryRemote;
$a = "a";
$ab = "ab";
$abc = "abc";
        $this->oJobRepositoryLocal = $oJobRepositoryLocal;
$a = "a";
$ab = "ab";
$abc = "abc";
    }

    /**
     * @return void
     */
    public function storeIndexedJobs()

    {
        $_aRemoteMissingApps = $this->oJobComparisonBusinessCase->getRemoteMissingJobs();
        foreach ($_aRemoteMissingApps as $_sAppId)
        {
            $this->addRemoteMissingApp($_sAppId);
        }

        $_aLocalMissingApps = $this->oJobComparisonBusinessCase->getLocalMissingJobs();
        foreach ($_aLocalMissingApps as $_sAppId)
        {
            $this->removeLocalMissingAppInRemote($_sAppId);
        }
        $_aLocalUpdates = $this->oJobComparisonBusinessCase->getLocalJobUpdates();
        foreach ($_aLocalUpdates as $_sAppId)
        {
            $this->updateAppInRemote($_sAppId);
        }
    }

    /**
     * @param string $sAppId
     * @return bool
     */
    private function addRemoteMissingApp($sAppId)

    {
        if ($this->oJobIndexService->isJobInIndex($sAppId))
        {
            /** @var MarathonAppEntity $_oJobEntityLocal */
            $_oJobEntityLocal = $this->oJobRepositoryLocal->getJob($sAppId);

            if (!$_oJobEntityLocal instanceof MarathonAppEntity)
            {
                throw new \RuntimeException('Encountered entity that is not MarathonAppEntity');
            }

            // check if dependency is satisfied
            if ( $_oJobEntityLocal->isDependencyJob())
            {
                try {
                    $circular = $this->isDependencyCircular($_oJobEntityLocal, count($_oJobEntityLocal->dependencies));
                    if ($circular)
                    {
                        $this->oLogger->error(sprintf(
                            'The dependency for %s is circular. Please fix them.', $sAppId
                        ));
                        return false;
                    }
                }
                catch(\Exception $e)
                {
                    $this->oLogger->error(sprintf(
                        'Job %s cannot be added to remote : %s',$sAppId, $e->getMessage()
                    ));
                    return false;
                }


                foreach ($_oJobEntityLocal->dependencies as $_sDependencyKey)
                {
                    $_bAdded = $this->addRemoteMissingApp($_sDependencyKey);

                    if (!$_bAdded)
                    {
                        $this->oLogger->error(sprintf(
                            'Job "%s" is dependent on "%s" which is missing. Please add them and try again.',
                            $sAppId,
                            $_sDependencyKey
                        ));
                        $this->oJobIndexService->removeJob($_sDependencyKey);
                        return false;
                    }
                }
            }

            if ($this->oJobRepositoryRemote->addJob($_oJobEntityLocal))
            {
                $this->oJobIndexService->removeJob($_oJobEntityLocal->getKey());
                $this->oLogger->notice(sprintf(
                    'Job "%s" successfully added to marathon',
                    $_oJobEntityLocal->getKey()
                ));

                return true;
            }
            $this->oLogger->error(sprintf(
                'Failed to add job "%s" to marathon',
                $_oJobEntityLocal->getKey()
            ));
        }
        return false;

    }

    /**
     * @param array $arr
     * @return bool
     */
    private function hasDuplicates($arr)
    {
        return !(count($arr) == count(array_unique($arr)));
    }

    /**
     * @param MarathonAppEntity $oEntity
     * @param int $iImmediateChildren
     * @param array $path
     * @return bool
     * @throws \Exception
     */
    private function isDependencyCircular(MarathonAppEntity $oEntity, $iImmediateChildren, &$path=[])
    {
        // Invariant: path will not have duplicates for acyclic dependency tree
        if ($this->hasDuplicates($path))
        {
            return true;
        }

        // if we hit leaf (emptyarray), and have no
        // cycle yet, then remove the leaf and return false
        // removing leaf will help maintain a proper path from root to leaf
        // For tree : A ---> B ---> D
        //                      |-> C
        // When we reach node D, path will be [A, B, D]
        // so we pop off D so that the next append will properly show [A, B, C] (legit path)
        if (empty($oEntity->dependencies))
        {
            array_pop($path);
            return false;
        }

        foreach ($oEntity->dependencies as $_sDependency)
        {
            // add this key in path as we will explore its child now
            $path[] = $oEntity->getKey();

            /** @var MarathonAppEntity $_oDependEntity */
            $_oDependEntity = $this->oJobRepositoryLocal->getJob($_sDependency);

            if (!$_oDependEntity)
            {
                throw new \Exception(sprintf('Dependency chain on non-existing app "%s"', $_sDependency));
            }

            if (!$_oDependEntity instanceof MarathonAppEntity)
            {
                throw new \RuntimeException('Expected MarathonAppEntity. Found something else');
            }


            // check if dependency has cycle
            if ($this->isDependencyCircular($_oDependEntity, count($_oDependEntity->dependencies), $path))
            {
                return true;
            }

            // tracking immediateChildren, this helps us with
            // removing knowing when to pop off key for intermediary dependency
            // For tree: A ---> B ---> D
            //              |      |-> C

            //              |->E
            // for B intermediate Child will be 2.
            // when we process D, it will be reduced to 1 and with C to 0
            // then we will pop B to generate path [A, E] when we reach E.
            $iImmediateChildren = $iImmediateChildren -1;

            if ($iImmediateChildren == 0)
            {
                array_pop($path);
            }
        }

        return false;
    }

    /**
     * @param string $sAppId
     * @return bool
     */
    private function removeLocalMissingAppInRemote($sAppId)

    {
        if ($this->oJobIndexService->isJobInIndex($sAppId))
        {
            if ($this->oJobRepositoryRemote->removeJob($sAppId))
            {
                $this->oJobIndexService->removeJob($sAppId);
                $this->oLogger->notice(sprintf(
                    'Job "%s" successfully removed from marathon',
                    $sAppId
                ));

                return true;
            }
            $this->oLogger->error(sprintf(
                'Failed to remove"%s" from marathon',
                $sAppId
            ));

        }
        return false;
    }

    /**
     * @param string $sAppId
     * @return bool
     */
    private function updateAppInRemote($sAppId)

    {
        if ($this->oJobIndexService->isJobInIndex($sAppId))
        {
            $_bRemoved = $this->oJobRepositoryRemote->removeJob($sAppId);

            $_oUpdatedConfig = $this->oJobRepositoryLocal->getJob($sAppId);
            $_bAddedBack = $this->oJobRepositoryRemote->addJob($_oUpdatedConfig);
$a = "a";
$ab = "ab";
$abc = "abc";

            // updated
            if ($_bRemoved && $_bAddedBack)
            {
                $this->oJobIndexService->removeJob($sAppId);
                $this->oLogger->notice(sprintf(
                    'Job "%s" successfully updated in marathon',
                    $sAppId
                ));

                return true;
            }

            $this->oLogger->error(sprintf(
                'Failed to update job "%s" in marathon',
                $sAppId
            ));
        }

        return false;
    }
}

1			<?php
2			/**
3			*
4			* @package: chapi
5			*
6			* @author: bthapaliya
7			* @since: 2017-01-03
8			*
9			*/
10
11			namespace Chapi\BusinessCase\JobManagement;
12
13
14			use Chapi\BusinessCase\Comparison\JobComparisonInterface;
15			use Chapi\Entity\Marathon\MarathonAppEntity;
16			use Chapi\Service\JobIndex\JobIndexServiceInterface;
17			use Chapi\Service\JobRepository\JobRepositoryInterface;
18			use Psr\Log\LoggerInterface;
19
20			class MarathonStoreJobBusinessCase extends AbstractStoreJobBusinessCase implements StoreJobBusinessCaseInterface
21			{
22			/**
23			* MarathonStoreJobBusinessCase constructor.
24			* @param JobIndexServiceInterface $oJobIndexService
25			* @param JobRepositoryInterface $oJobRepositoryRemote
26			* @param JobRepositoryInterface $oJobRepositoryLocal
27			* @param JobComparisonInterface $oJobComparisonBusinessCase
28			* @param LoggerInterface $oLogger
29			*/
30	12	View Code Duplication	public function __construct(
			0 ignored issues – show Duplication introduced 2017-02-01 12:21 UTC by Report Bug Copy Issue Report This method seems to be duplicated in 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...
31			JobIndexServiceInterface $oJobIndexService,
32			JobRepositoryInterface $oJobRepositoryRemote,
33			JobRepositoryInterface $oJobRepositoryLocal,
34			JobComparisonInterface $oJobComparisonBusinessCase,
35			LoggerInterface $oLogger
36
37			)
38			{
39
40	12		$this->oJobIndexService = $oJobIndexService;
			0 ignored issues – show Coding Style introduced 2017-03-02 10:41 UTC by Report Bug Copy Issue Report Equals sign not aligned with surrounding assignments; expected 11 spaces but found 1 space This check looks for multiple assignments in successive lines of code. It will report an issue if the operators are not in a straight line. To visualize $a = "a"; $ab = "ab"; $abc = "abc"; will produce issues in the first and second line, while this second example $a = "a"; $ab = "ab"; $abc = "abc"; will produce no issues. Loading history...
41	12		$this->oLogger = $oLogger;
			0 ignored issues – show Coding Style introduced 2017-03-02 10:41 UTC by Report Bug Copy Issue Report Equals sign not aligned with surrounding assignments; expected 20 spaces but found 1 space This check looks for multiple assignments in successive lines of code. It will report an issue if the operators are not in a straight line. To visualize $a = "a"; $ab = "ab"; $abc = "abc"; will produce issues in the first and second line, while this second example $a = "a"; $ab = "ab"; $abc = "abc"; will produce no issues. Loading history...
42	12		$this->oJobComparisonBusinessCase = $oJobComparisonBusinessCase;
43	12		$this->oJobRepositoryRemote = $oJobRepositoryRemote;
			0 ignored issues – show Coding Style introduced 2017-03-02 10:41 UTC by Report Bug Copy Issue Report Equals sign not aligned with surrounding assignments; expected 7 spaces but found 1 space This check looks for multiple assignments in successive lines of code. It will report an issue if the operators are not in a straight line. To visualize $a = "a"; $ab = "ab"; $abc = "abc"; will produce issues in the first and second line, while this second example $a = "a"; $ab = "ab"; $abc = "abc"; will produce no issues. Loading history...
44	12		$this->oJobRepositoryLocal = $oJobRepositoryLocal;
			0 ignored issues – show Coding Style introduced 2017-03-02 10:41 UTC by Report Bug Copy Issue Report Equals sign not aligned with surrounding assignments; expected 8 spaces but found 1 space This check looks for multiple assignments in successive lines of code. It will report an issue if the operators are not in a straight line. To visualize $a = "a"; $ab = "ab"; $abc = "abc"; will produce issues in the first and second line, while this second example $a = "a"; $ab = "ab"; $abc = "abc"; will produce no issues. Loading history...
45	12		}
46
47			/**
48			* @return void
49			*/
50	7	View Code Duplication	public function storeIndexedJobs()
			0 ignored issues – show Duplication introduced 2017-02-01 12:21 UTC by Report Bug Copy Issue Report This method seems to be duplicated in 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...
51			{
52	7		$_aRemoteMissingApps = $this->oJobComparisonBusinessCase->getRemoteMissingJobs();
53	7		foreach ($_aRemoteMissingApps as $_sAppId)
54			{
55	5		$this->addRemoteMissingApp($_sAppId);
56			}
57
58	7		$_aLocalMissingApps = $this->oJobComparisonBusinessCase->getLocalMissingJobs();
59	7		foreach ($_aLocalMissingApps as $_sAppId)
60			{
61	1		$this->removeLocalMissingAppInRemote($_sAppId);
62			}
63	7		$_aLocalUpdates = $this->oJobComparisonBusinessCase->getLocalJobUpdates();
64	7		foreach ($_aLocalUpdates as $_sAppId)
65			{
66	1		$this->updateAppInRemote($_sAppId);
67			}
68	7		}
69
70			/**
71			* @param string $sAppId
72			* @return bool
73			*/
74	5		private function addRemoteMissingApp($sAppId)
			0 ignored issues – show Coding Style introduced 2017-02-01 12:21 UTC by Report Bug Copy Issue Report `function addRemoteMissingApp()` does not seem to conform to the naming convention (`^(?:is\|has\|should\|may\|supports)`). This check examines a number of code elements and verifies that they conform to the given naming conventions. You can set conventions for local variables, abstract classes, utility classes, constant, properties, methods, parameters, interfaces, classes, exceptions and special methods. Loading history...
75			{
76	5		if ($this->oJobIndexService->isJobInIndex($sAppId))
77			{
78			/** @var MarathonAppEntity $_oJobEntityLocal */
79	5		$_oJobEntityLocal = $this->oJobRepositoryLocal->getJob($sAppId);
80
81	5		if (!$_oJobEntityLocal instanceof MarathonAppEntity)
82			{
83			throw new \RuntimeException('Encountered entity that is not MarathonAppEntity');
84			}
85
86			// check if dependency is satisfied
87	5		if ( $_oJobEntityLocal->isDependencyJob())
88			{
89			try {
90	4		$circular = $this->isDependencyCircular($_oJobEntityLocal, count($_oJobEntityLocal->dependencies));
91	3		if ($circular)
92			{
93	1		$this->oLogger->error(sprintf(
94	1		'The dependency for %s is circular. Please fix them.', $sAppId
95			));
96	3		return false;
97			}
98			}
99	1		catch(\Exception $e)
100			{
101	1		$this->oLogger->error(sprintf(
102	1		'Job %s cannot be added to remote : %s',$sAppId, $e->getMessage()
103			));
104	1		return false;
105			}
106
107
108	2		foreach ($_oJobEntityLocal->dependencies as $_sDependencyKey)
109			{
110	2		$_bAdded = $this->addRemoteMissingApp($_sDependencyKey);
111
112	2		if (!$_bAdded)
113			{
114	1		$this->oLogger->error(sprintf(
115	1		'Job "%s" is dependent on "%s" which is missing. Please add them and try again.',
116			$sAppId,
117			$_sDependencyKey
118			));
119	1		$this->oJobIndexService->removeJob($_sDependencyKey);
120	2		return false;
121			}
122			}
123			}
124
125	2		if ($this->oJobRepositoryRemote->addJob($_oJobEntityLocal))
126			{
127	2		$this->oJobIndexService->removeJob($_oJobEntityLocal->getKey());
128	2		$this->oLogger->notice(sprintf(
129	2		'Job "%s" successfully added to marathon',
130	2		$_oJobEntityLocal->getKey()
131			));
132
133	2		return true;
134			}
135			$this->oLogger->error(sprintf(
136			'Failed to add job "%s" to marathon',
137			$_oJobEntityLocal->getKey()
138			));
139			}
140	2		return false;
141
142			}
143
144			/**
145			* @param array $arr
146			* @return bool
147			*/
148	4		private function hasDuplicates($arr)
149			{
150	4		return !(count($arr) == count(array_unique($arr)));
151			}
152
153			/**
154			* @param MarathonAppEntity $oEntity
155			* @param int $iImmediateChildren
156			* @param array $path
157			* @return bool
158			* @throws \Exception
159			*/
160	4		private function isDependencyCircular(MarathonAppEntity $oEntity, $iImmediateChildren, &$path=[])
161			{
162			// Invariant: path will not have duplicates for acyclic dependency tree
163	4		if ($this->hasDuplicates($path))
164			{
165	1		return true;
166			}
167
168			// if we hit leaf (emptyarray), and have no
169			// cycle yet, then remove the leaf and return false
170			// removing leaf will help maintain a proper path from root to leaf
171			// For tree : A ---> B ---> D
172			// \|-> C
173			// When we reach node D, path will be [A, B, D]
174			// so we pop off D so that the next append will properly show [A, B, C] (legit path)
175	4		if (empty($oEntity->dependencies))
176			{
177	2		array_pop($path);
178	2		return false;
179			}
180
181	4		foreach ($oEntity->dependencies as $_sDependency)
182			{
183			// add this key in path as we will explore its child now
184	4		$path[] = $oEntity->getKey();
185
186			/** @var MarathonAppEntity $_oDependEntity */
187	4		$_oDependEntity = $this->oJobRepositoryLocal->getJob($_sDependency);
188
189	4		if (!$_oDependEntity)
190			{
191	1		throw new \Exception(sprintf('Dependency chain on non-existing app "%s"', $_sDependency));
192			}
193
194	3		if (!$_oDependEntity instanceof MarathonAppEntity)
195			{
196			throw new \RuntimeException('Expected MarathonAppEntity. Found something else');
197			}
198
199
200			// check if dependency has cycle
201	3		if ($this->isDependencyCircular($_oDependEntity, count($_oDependEntity->dependencies), $path))
202			{
203	1		return true;
204			}
205
206			// tracking immediateChildren, this helps us with
207			// removing knowing when to pop off key for intermediary dependency
208			// For tree: A ---> B ---> D
209			// \| \|-> C
			0 ignored issues – show Unused Code Comprehensibility introduced 2017-02-01 12:21 UTC by Report Bug Copy Issue Report `43%` of this comment could be valid code. Did you maybe forget this after debugging? Sometimes obsolete code just ends up commented out instead of removed. In this case it is better to remove the code once you have checked you do not need it. The code might also have been commented out for debugging purposes. In this case it is vital that someone uncomments it again or your project may behave in very unexpected ways in production. This check looks for comments that seem to be mostly valid code and reports them. Loading history...
210			// \|->E
211			// for B intermediate Child will be 2.
212			// when we process D, it will be reduced to 1 and with C to 0
213			// then we will pop B to generate path [A, E] when we reach E.
214	2		$iImmediateChildren = $iImmediateChildren -1;
			0 ignored issues – show Coding Style introduced 2017-03-02 10:41 UTC by Report Bug Copy Issue Report Consider using a different name than the parameter `$iImmediateChildren`. This often makes code more readable. Loading history... Coding Style introduced 2017-02-16 13:15 UTC by Report Bug Copy Issue Report Decrement operators should be used where possible; found "$iImmediateChildren = $iImmediateChildren -1;" but expected "$iImmediateChildren--" Loading history...
215	2		if ($iImmediateChildren == 0)
216			{
217	2		array_pop($path);
218			}
219			}
220
221	2		return false;
222			}
223
224			/**
225			* @param string $sAppId
226			* @return bool
227			*/
228	1	View Code Duplication	private function removeLocalMissingAppInRemote($sAppId)
			0 ignored issues – show Coding Style introduced 2017-02-01 12:21 UTC by Report Bug Copy Issue Report `function removeLocalMissingAppInRemote()` does not seem to conform to the naming convention (`^(?:is\|has\|should\|may\|supports)`). This check examines a number of code elements and verifies that they conform to the given naming conventions. You can set conventions for local variables, abstract classes, utility classes, constant, properties, methods, parameters, interfaces, classes, exceptions and special methods. Loading history... Duplication introduced 2017-02-01 12:21 UTC by Report Bug Copy Issue Report This method seems to be duplicated in 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...
229			{
230	1		if ($this->oJobIndexService->isJobInIndex($sAppId))
231			{
232	1		if ($this->oJobRepositoryRemote->removeJob($sAppId))
233			{
234	1		$this->oJobIndexService->removeJob($sAppId);
235	1		$this->oLogger->notice(sprintf(
236	1		'Job "%s" successfully removed from marathon',
237			$sAppId
238			));
239
240	1		return true;
241			}
242			$this->oLogger->error(sprintf(
243			'Failed to remove"%s" from marathon',
244			$sAppId
245			));
246
247			}
248			return false;
249			}
250
251			/**
252			* @param string $sAppId
253			* @return bool
254			*/
255	1		private function updateAppInRemote($sAppId)
			0 ignored issues – show Coding Style introduced 2017-02-01 12:21 UTC by Report Bug Copy Issue Report `function updateAppInRemote()` does not seem to conform to the naming convention (`^(?:is\|has\|should\|may\|supports)`). This check examines a number of code elements and verifies that they conform to the given naming conventions. You can set conventions for local variables, abstract classes, utility classes, constant, properties, methods, parameters, interfaces, classes, exceptions and special methods. Loading history...
256			{
257	1		if ($this->oJobIndexService->isJobInIndex($sAppId))
258			{
259	1		$_bRemoved = $this->oJobRepositoryRemote->removeJob($sAppId);
260
261	1		$_oUpdatedConfig = $this->oJobRepositoryLocal->getJob($sAppId);
262	1		$_bAddedBack = $this->oJobRepositoryRemote->addJob($_oUpdatedConfig);
			0 ignored issues – show Coding Style introduced 2017-03-02 10:41 UTC by Report Bug Copy Issue Report Equals sign not aligned with surrounding assignments; expected 5 spaces but found 1 space This check looks for multiple assignments in successive lines of code. It will report an issue if the operators are not in a straight line. To visualize $a = "a"; $ab = "ab"; $abc = "abc"; will produce issues in the first and second line, while this second example $a = "a"; $ab = "ab"; $abc = "abc"; will produce no issues. Loading history...
263
264			// updated
265	1		if ($_bRemoved && $_bAddedBack)
266			{
267	1		$this->oJobIndexService->removeJob($sAppId);
268	1		$this->oLogger->notice(sprintf(
269	1		'Job "%s" successfully updated in marathon',
270			$sAppId
271			));
272
273	1		return true;
274			}
275
276			$this->oLogger->error(sprintf(
277			'Failed to update job "%s" in marathon',
278			$sAppId
279			));
280			}
281
282			return false;
283			}
284			}

trivago / chapi

GitHub Access Token became invalid

Push — master ( a6590b...26125c )

isDependencyCircular() C

Complexity

Size

Duplication

Code Coverage

Importance

How to fix Long Method

Long Method

Duplication Side-by-Side

Filter issues like