RowCollection - Code Metrics - Inspection of "added RowCollection::find()" - oscarotero/simple-crud - Measure and Improve Code Quality continuously with Scrutinizer

Completed

Push — master ( d7c3e2...8885dc )

by Oscar

created 2015-12-08 10:35 UTC

RowCollection C

↳ Parent: Project

Complexity

Total Complexity

Size/Duplication

Total Lines	398
Duplicated Lines	6.03 %

Coupling/Cohesion

Components	1
Dependencies	5

Importance

Changes	45
Bugs	7	Features	7

Metric	Value
wmc	70
c	45
b	7
f	7
lcom	1
cbo	5
dl	24
loc	398
rs	5.6164

23 Methods

Rating	Name	Duplication	Size	Complexity
A	__set()	0	4	1
C	__get()	0	47	9
A	idAsKey()	0	6	1
A	__toString()	0	4	1
A	offsetSet()	0	18	4
A	offsetExists()	0	4	1
A	offsetUnset()	0	4	1
A	offsetGet()	0	4	2
A	rewind()	0	4	1
A	current()	0	4	1
A	key()	0	4	1
A	next()	0	4	1
A	valid()	0	4	1
A	count()	0	4	1
B	toArray()	0	14	5
A	set()	0	8	2
D	get()	14	42	10
A	slice()	0	8	2
B	add()	0	12	5
B	filter()	5	12	7
B	find()	5	8	7
B	joinMany()	0	26	5
A	joinOne()	0	6	1

How to fix Duplicated Code Complexity

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:

If you have the same expression in different places: Extract expression to a method
If you have the same method in different sub-classes: Extract method, and pull up field to the parent class
If you have the same code in unrelated classes: Consider extracting the code to a new class, and injecting that class

Complex Class

Tip: Before tackling complexity, make sure that you eliminate any duplication first. This often can reduce the size of classes significantly.

Complex classes like RowCollection often do a lot of different things. To break such a class down, we need to identify a cohesive component within that class. A common approach to find such a component is to look for fields/methods that share the same prefixes, or suffixes. You can also have a look at the cohesion graph to spot any un-connected, or weakly-connected components.

Once you have determined the fields that belong together, you can apply the Extract Class refactoring. If the component makes sense as a sub-class, Extract Subclass is also a candidate, and is often faster.

While breaking up the class, it is a good idea to analyze how other classes use RowCollection, and based on these observations, apply Extract Interface, too.

<?php

namespace SimpleCrud;

use ArrayAccess;
use Iterator;
use Countable;

/**
 * Stores a collection of rows.
 */
class RowCollection extends BaseRow implements ArrayAccess, Iterator, Countable
{
    private $rows = [];
    private $idAsKey = true;

    /**
     * Magic method to set properties to all rows.
     *
     * @see self::set()
     */
    public function __set($name, $value)
    {
        $this->set($name, $value);
    }

    /**
     * Magic method to get properties from all rows.
     *
     * @see self::get()
     */
    public function __get($name)
    {
        reset($this->rows);
        $first = current($this->rows);

        if (!$first) {
            return [];
        }

        //Returns related entities
        $db = $this->entity->getDb();

        if ($db->has($name)) {
            $entity = $db->get($name);

            if ($first->has($name)) {
                $collection = $entity->createCollection();

                foreach ($this->get($name) as $row) {
                    if ($row instanceof self) {
                        foreach ($row as $r) {
                            $collection[] = $r;
                        }
                    } else {
                        $collection = $row;
                    }
                }

                return $collection;
            }

            $collection = $this->select($name)->all(false);
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.
    }
}

            if ($this->entity->hasOne($entity)) {
/** @return stdClass|null */
function mayReturnNull() { }

function doesNotAcceptNull(stdClass $x) { }

// With potential error.
function withoutCheck() {
    $x = mayReturnNull();
    doesNotAcceptNull($x); // Potential error here.
}

// Safe - Alternative 1
function withCheck1() {
    $x = mayReturnNull();
    if ( ! $x instanceof stdClass) {
        throw new \LogicException('$x must be defined.');
    }
    doesNotAcceptNull($x);
}

// Safe - Alternative 2
function withCheck2() {
    $x = mayReturnNull();
    if ($x instanceof stdClass) {
        doesNotAcceptNull($x);
    }
}
                $this->joinOne($collection);
            } else {
                $this->joinMany($collection);
            }

            return $collection;
        }

        //Returns values
        if ($first->has($name)) {
            return $this->get($name);
        }
    }

    /**
     * Set whether or not use the id as key.
     *
     * @param bool $idAsKey
     *
     * @return self
     */
    public function idAsKey($idAsKey)
    {
        $this->idAsKey = (boolean) $idAsKey;

        return $this;
    }

    /**
     * Magic method to print the row values (and subvalues).
     *
     * @return string
     */
    public function __toString()
    {
        return "\n".$this->entity->name.":\n".print_r($this->toArray(), true)."\n";
    }

    /**
     * @see ArrayAccess
     */
    public function offsetSet($offset, $value)
    {
        if (!($value instanceof Row)) {
            throw new SimpleCrudException('Only instances of SimpleCrud\\Row must be added to collections');
        }

        if ($this->idAsKey === false) {
            $this->rows[] = $value;

            return;
        }

        if (empty($value->id)) {
            throw new SimpleCrudException('Only rows with the defined id must be added to collections');
        }

        $this->rows[$value->id] = $value;
    }

    /**
     * @see ArrayAccess
     */
    public function offsetExists($offset)
    {
        return isset($this->rows[$offset]);
    }

    /**
     * @see ArrayAccess
     */
    public function offsetUnset($offset)
    {
        unset($this->rows[$offset]);
    }

    /**
     * @see ArrayAccess
     */
    public function offsetGet($offset)
    {
        return isset($this->rows[$offset]) ? $this->rows[$offset] : null;
    }

    /**
     * @see Iterator
     */
    public function rewind()
    {
        return reset($this->rows);
    }

    /**
     * @see Iterator
     */
    public function current()
    {
        return current($this->rows);
    }

    /**
     * @see Iterator
     */
    public function key()
    {
        return key($this->rows);
    }

    /**
     * @see Iterator
     */
    public function next()
    {
        return next($this->rows);
    }

    /**
     * @see Iterator
     */
    public function valid()
    {
        return key($this->rows) !== null;
    }

    /**
     * @see Countable
     */
    public function count()
    {
        return count($this->rows);
    }

    /**
     * {@inheritdoc}
     */
    public function toArray($idAsKey = false, array $parentEntities = array())
    {
        if (!empty($parentEntities) && in_array($this->entity->name, $parentEntities)) {
            return;
        }

        $rows = [];

        foreach ($this->rows as $id => $row) {
            $rows[$id] = $row->toArray($parentEntities);
        }

        return $idAsKey ? $rows : array_values($rows);
    }

    /**
     * Set values to all children.
     *
     * @param string $name
     * @param string $value
     *
     * @return self
     */
    public function set($name, $value)
    {
        foreach ($this->rows as $row) {
            $row->$name = $value;
        }

        return $this;
    }

    /**
     * Returns one or all values of the collections.
     *
     * @param string $name The value name. If it's not defined returns all values
     * @param string $key  The parameter name used for the keys. If it's not defined, returns a numeric array
     *
     * @return array
     */
    public function get($name = null, $key = null)
    {
        $rows = [];

        if ($name === null) {
            if ($key === null) {
                return array_values($this->rows);
            }

            foreach ($this->rows as $row) {

                $k = $row->$key;

                if (!empty($k)) {
                    $rows[$k] = $row;
                }
            }

            return $rows;
        }

        if ($key !== null) {
            foreach ($this->rows as $row) {

                $k = $row->$key;

                if (!empty($k)) {
                    $rows[$k] = $row->$name;
                }
            }

            return $rows;
        }

        foreach ($this->rows as $row) {
            $value = $row->$name;

            if (!empty($value)) {
                $rows[] = $value;
            }
        }

        return $rows;
    }

    /**
     * Returns a slice of the content.
     *
     * @param int           $offset
     * @param int|null|true $length
     *
     * @return array
     */
    public function slice($offset = null, $length = null)
    {
        if ($length === true) {
            return current(array_slice($this->rows, $offset, 1));
        }

        return array_slice($this->rows, $offset, $length);
    }

    /**
     * Add new values to the collection.
     *
     * @param array|RowInterface $rows The new rows
     *
     * @return $this
     */
    public function add($rows)
    {
        if (is_array($rows) || ($rows instanceof self)) {
            foreach ($rows as $row) {
                $this->offsetSet(null, $row);
            }
        } elseif (isset($rows)) {
            $this->offsetSet(null, $rows);
        }

        return $this;
    }

    /**
     * Filter the rows by a value.
     *
     * @param string $name   The value name
     * @param mixed  $value  The value to filter
     * @param bool   $strict Strict mode
     *
     * @return RowCollection
     */
    public function filter($name, $value, $strict = true)
    {
        $rows = [];

        foreach ($this->rows as $row) {

            if (($row->$name === $value) || (!$strict && $row->$name == $value) || (is_array($value) && in_array($row->$name, $value, $strict))) {
                $rows[] = $row;
            }
        }

        return $this->entity->createCollection($rows);
    }

    /**
     * Find a row by a value.
     *
     * @param string $name   The value name
     * @param mixed  $value  The value to filter
     * @param bool   $strict Strict mode
     *
     * @return Row|null The rows found
     */
    public function find($name, $value, $strict = true)
    {
        foreach ($this->rows as $row) {

            if (($row->$name === $value) || (!$strict && $row->$name == $value) || (is_array($value) && in_array($row->$name, $value, $strict))) {
                return $row;
            }
        }
    }

    /**
     * Distribute a rowcollection througth all rows.
     *
     * @param RowCollection $rows
     *
     * @return $this
     */
    public function joinMany(RowCollection $rows)
    {
        $thisEntity = $this->entity;
        $thatEntity = $rows->getEntity();
        $thisName = $thisEntity->name;
        $thatName = $thatEntity->name;

        $foreignKey = $thisEntity->foreignKey;

        foreach ($this->rows as $row) {
            if (!isset($row->$thatName)) {
                $row->$thatName = $thatEntity->createCollection();
            }
        }

        foreach ($rows as $row) {
            $id = $row->$foreignKey;

            if (isset($this->rows[$id])) {
                $this->rows[$id]->$thatName->add($row);
                $row->$thisName = $this->rows[$id];
            }
        }

        return $this;
    }

    /**
     * Distribute a rowcollection througth all rows.
     * Its the opposite of $this->joinMany().
     *
     * @param RowCollection $rows
     *
     * @return $this
     */
    public function joinOne(RowCollection $rows)
    {
        $rows->joinMany($this);

        return $this;
    }
}


1		<?php
2
3		namespace SimpleCrud;
4
5		use ArrayAccess;
6		use Iterator;
7		use Countable;
8
9		/**
10		* Stores a collection of rows.
11		*/
12		class RowCollection extends BaseRow implements ArrayAccess, Iterator, Countable
13		{
14		private $rows = [];
15		private $idAsKey = true;
16
17		/**
18		* Magic method to set properties to all rows.
19		*
20		* @see self::set()
21		*/
22		public function __set($name, $value)
23		{
24		$this->set($name, $value);
25		}
26
27		/**
28		* Magic method to get properties from all rows.
29		*
30		* @see self::get()
31		*/
32		public function __get($name)
33		{
34		reset($this->rows);
35		$first = current($this->rows);
36
37		if (!$first) {
38		return [];
39		}
40
41		//Returns related entities
42		$db = $this->entity->getDb();
43
44		if ($db->has($name)) {
45		$entity = $db->get($name);
46
47		if ($first->has($name)) {
48		$collection = $entity->createCollection();
49
50		foreach ($this->get($name) as $row) {
51		if ($row instanceof self) {
52		foreach ($row as $r) {
53		$collection[] = $r;
54		}
55		} else {
56		$collection = $row;
57		}
58		}
59
60		return $collection;
61		}
62
63		$collection = $this->select($name)->all(false);
		0 ignored issues – show Bug introduced 2015-12-02 18:20 UTC by Report Bug Copy Issue Report It seems like you code against a concrete implementation and not the interface `SimpleCrud\QueryInterface` as the method `all()` does only exist in the following implementations of said interface: `SimpleCrud\Queries\Mysql\Select`, `SimpleCrud\Queries\Sqlite\Select`. 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 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 interface: interface User { /** @return string / public function getPassword(); /* @return string */ public function getDisplayName(); } Loading history...
64
65		if ($this->entity->hasOne($entity)) {
		0 ignored issues – show Bug introduced 2015-12-02 18:20 UTC by Report Bug Copy Issue Report It seems like `$entity` defined by `$db->get($name)` on line `45` can be `null`; however, `SimpleCrud\Entity::hasOne()` does not accept `null`, maybe add an additional type check? Unless you are absolutely sure that the expression can never be null because of other conditions, we strongly recommend to add an additional type check to your code: /** @return stdClass\|null */ function mayReturnNull() { } function doesNotAcceptNull(stdClass $x) { } // With potential error. function withoutCheck() { $x = mayReturnNull(); doesNotAcceptNull($x); // Potential error here. } // Safe - Alternative 1 function withCheck1() { $x = mayReturnNull(); if ( ! $x instanceof stdClass) { throw new \LogicException('$x must be defined.'); } doesNotAcceptNull($x); } // Safe - Alternative 2 function withCheck2() { $x = mayReturnNull(); if ($x instanceof stdClass) { doesNotAcceptNull($x); } } Loading history...
66		$this->joinOne($collection);
67		} else {
68		$this->joinMany($collection);
69		}
70
71		return $collection;
72		}
73
74		//Returns values
75		if ($first->has($name)) {
76		return $this->get($name);
77		}
78		}
79
80		/**
81		* Set whether or not use the id as key.
82		*
83		* @param bool $idAsKey
84		*
85		* @return self
86		*/
87		public function idAsKey($idAsKey)
88		{
89		$this->idAsKey = (boolean) $idAsKey;
90
91		return $this;
92		}
93
94		/**
95		* Magic method to print the row values (and subvalues).
96		*
97		* @return string
98		*/
99		public function __toString()
100		{
101		return "\n".$this->entity->name.":\n".print_r($this->toArray(), true)."\n";
102		}
103
104		/**
105		* @see ArrayAccess
106		*/
107		public function offsetSet($offset, $value)
108		{
109		if (!($value instanceof Row)) {
110		throw new SimpleCrudException('Only instances of SimpleCrud\\Row must be added to collections');
111		}
112
113		if ($this->idAsKey === false) {
114		$this->rows[] = $value;
115
116		return;
117		}
118
119		if (empty($value->id)) {
120		throw new SimpleCrudException('Only rows with the defined id must be added to collections');
121		}
122
123		$this->rows[$value->id] = $value;
124		}
125
126		/**
127		* @see ArrayAccess
128		*/
129		public function offsetExists($offset)
130		{
131		return isset($this->rows[$offset]);
132		}
133
134		/**
135		* @see ArrayAccess
136		*/
137		public function offsetUnset($offset)
138		{
139		unset($this->rows[$offset]);
140		}
141
142		/**
143		* @see ArrayAccess
144		*/
145		public function offsetGet($offset)
146		{
147		return isset($this->rows[$offset]) ? $this->rows[$offset] : null;
148		}
149
150		/**
151		* @see Iterator
152		*/
153		public function rewind()
154		{
155		return reset($this->rows);
156		}
157
158		/**
159		* @see Iterator
160		*/
161		public function current()
162		{
163		return current($this->rows);
164		}
165
166		/**
167		* @see Iterator
168		*/
169		public function key()
170		{
171		return key($this->rows);
172		}
173
174		/**
175		* @see Iterator
176		*/
177		public function next()
178		{
179		return next($this->rows);
180		}
181
182		/**
183		* @see Iterator
184		*/
185		public function valid()
186		{
187		return key($this->rows) !== null;
188		}
189
190		/**
191		* @see Countable
192		*/
193		public function count()
194		{
195		return count($this->rows);
196		}
197
198		/**
199		* {@inheritdoc}
200		*/
201		public function toArray($idAsKey = false, array $parentEntities = array())
202		{
203		if (!empty($parentEntities) && in_array($this->entity->name, $parentEntities)) {
204		return;
205		}
206
207		$rows = [];
208
209		foreach ($this->rows as $id => $row) {
210		$rows[$id] = $row->toArray($parentEntities);
211		}
212
213		return $idAsKey ? $rows : array_values($rows);
214		}
215
216		/**
217		* Set values to all children.
218		*
219		* @param string $name
220		* @param string $value
221		*
222		* @return self
223		*/
224		public function set($name, $value)
225		{
226		foreach ($this->rows as $row) {
227		$row->$name = $value;
228		}
229
230		return $this;
231		}
232
233		/**
234		* Returns one or all values of the collections.
235		*
236		* @param string $name The value name. If it's not defined returns all values
237		* @param string $key The parameter name used for the keys. If it's not defined, returns a numeric array
238		*
239		* @return array
240		*/
241		public function get($name = null, $key = null)
242		{
243		$rows = [];
244
245		if ($name === null) {
246		if ($key === null) {
247		return array_values($this->rows);
248		}
249
250	View Code Duplication	foreach ($this->rows as $row) {
		0 ignored issues – show Duplication introduced 2015-12-02 18:20 UTC by Report Bug Copy Issue Report 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...
251		$k = $row->$key;
252
253		if (!empty($k)) {
254		$rows[$k] = $row;
255		}
256		}
257
258		return $rows;
259		}
260
261		if ($key !== null) {
262	View Code Duplication	foreach ($this->rows as $row) {
		0 ignored issues – show Duplication introduced 2015-12-02 18:20 UTC by Report Bug Copy Issue Report 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...
263		$k = $row->$key;
264
265		if (!empty($k)) {
266		$rows[$k] = $row->$name;
267		}
268		}
269
270		return $rows;
271		}
272
273		foreach ($this->rows as $row) {
274		$value = $row->$name;
275
276		if (!empty($value)) {
277		$rows[] = $value;
278		}
279		}
280
281		return $rows;
282		}
283
284		/**
285		* Returns a slice of the content.
286		*
287		* @param int $offset
288		* @param int\|null\|true $length
289		*
290		* @return array
291		*/
292		public function slice($offset = null, $length = null)
293		{
294		if ($length === true) {
295		return current(array_slice($this->rows, $offset, 1));
296		}
297
298		return array_slice($this->rows, $offset, $length);
299		}
300
301		/**
302		* Add new values to the collection.
303		*
304		* @param array\|RowInterface $rows The new rows
305		*
306		* @return $this
307		*/
308		public function add($rows)
309		{
310		if (is_array($rows) \|\| ($rows instanceof self)) {
311		foreach ($rows as $row) {
312		$this->offsetSet(null, $row);
313		}
314		} elseif (isset($rows)) {
315		$this->offsetSet(null, $rows);
316		}
317
318		return $this;
319		}
320
321		/**
322		* Filter the rows by a value.
323		*
324		* @param string $name The value name
325		* @param mixed $value The value to filter
326		* @param bool $strict Strict mode
327		*
328		* @return RowCollection
329		*/
330		public function filter($name, $value, $strict = true)
331		{
332		$rows = [];
333
334	View Code Duplication	foreach ($this->rows as $row) {
		0 ignored issues – show Duplication introduced 2015-12-08 10:40 UTC by Report Bug Copy Issue Report 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...
335		if (($row->$name === $value) \|\| (!$strict && $row->$name == $value) \|\| (is_array($value) && in_array($row->$name, $value, $strict))) {
336		$rows[] = $row;
337		}
338		}
339
340		return $this->entity->createCollection($rows);
341		}
342
343		/**
344		* Find a row by a value.
345		*
346		* @param string $name The value name
347		* @param mixed $value The value to filter
348		* @param bool $strict Strict mode
349		*
350		* @return Row\|null The rows found
351		*/
352		public function find($name, $value, $strict = true)
353		{
354	View Code Duplication	foreach ($this->rows as $row) {
		0 ignored issues – show Duplication introduced 2015-12-08 10:40 UTC by Report Bug Copy Issue Report 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...
355		if (($row->$name === $value) \|\| (!$strict && $row->$name == $value) \|\| (is_array($value) && in_array($row->$name, $value, $strict))) {
356		return $row;
357		}
358		}
359		}
360
361		/**
362		* Distribute a rowcollection througth all rows.
363		*
364		* @param RowCollection $rows
365		*
366		* @return $this
367		*/
368		public function joinMany(RowCollection $rows)
369		{
370		$thisEntity = $this->entity;
371		$thatEntity = $rows->getEntity();
372		$thisName = $thisEntity->name;
373		$thatName = $thatEntity->name;
374
375		$foreignKey = $thisEntity->foreignKey;
376
377		foreach ($this->rows as $row) {
378		if (!isset($row->$thatName)) {
379		$row->$thatName = $thatEntity->createCollection();
380		}
381		}
382
383		foreach ($rows as $row) {
384		$id = $row->$foreignKey;
385
386		if (isset($this->rows[$id])) {
387		$this->rows[$id]->$thatName->add($row);
388		$row->$thisName = $this->rows[$id];
389		}
390		}
391
392		return $this;
393		}
394
395		/**
396		* Distribute a rowcollection througth all rows.
397		* Its the opposite of $this->joinMany().
398		*
399		* @param RowCollection $rows
400		*
401		* @return $this
402		*/
403		public function joinOne(RowCollection $rows)
404		{
405		$rows->joinMany($this);
406
407		return $this;
408		}
409		}
410

oscarotero / simple-crud

Push — master ( d7c3e2...8885dc )

RowCollection C

Complexity

Size/Duplication

Coupling/Cohesion

Importance

23 Methods

How to fix Duplicated Code Complexity

Duplicated Code

Complex Class

Available Fixes

Duplication Side-by-Side

Filter issues like