Repository - Code Metrics - Inspection of "partial index implemented" - tarantool-php/mapper - Measure and Improve Code Quality continuously with Scrutinizer

Completed

Push — master ( e4e252...9e06e2 )

by Dmitry

created 2016-04-10 11:27 UTC

Repository B

↳ Parent: Project

Complexity

Total Complexity

Size/Duplication

Total Lines	243
Duplicated Lines	7.41 %

Coupling/Cohesion

Components	1
Dependencies	3

Test Coverage

Coverage

99.21%

Importance

Changes	33
Bugs	5	Features	9

Metric	Value
c	33
b	5
f	9
dl	18
loc	243
wmc	54
lcom	1
cbo	3
ccs	125
cts	126
cp	0.9921
rs	7.0642

11 Methods

Rating	Name	Duplication	Size	Complexity
A	__construct()	0	4	1
A	__call()	0	13	3
A	findOne()	0	4	1
D	create()	10	33	10
C	find()	8	60	18
A	knows()	0	4	1
A	remove()	0	7	1
C	save()	0	49	12
A	register()	0	11	4
B	generateId()	0	27	2
A	getType()	0	4	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 Repository 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 Repository, and based on these observations, apply Extract Interface, too.

<?php

namespace Tarantool\Mapper;

use BadMethodCallException;
use LogicException;

class Repository implements Contracts\Repository
{
    private $type;
    private $entities = [];
    private $keyMap = [];
    private $findCache = [];

    private $magicMethodRules = [
        'by' => false,
        'firstBy' => true,
        'oneBy' => true,
    ];

    public function __construct(Contracts\Type $type)
    {
        $this->type = $type;
    }

    public function create($data = null)
    {
        if ($data && !is_array($data)) {
            $properties = $this->getType()->getProperties();
            if (count($properties) == 2) {
                $data = [$properties[1] => $data];
            } else {
                throw new LogicException('Data should be array');
            }
        }
        if ($data) {
            $newData = [];
            foreach ($data as $k => $v) {

                if (!is_numeric($k)) {
                    $newData[$k] = $v;
                } else {
                    if ($v instanceof Contracts\Entity) {
                        $type = $this->type->getManager()->findRepository($v)->getType();
                        $newData[$this->type->getReferenceProperty($type)] = $v;
                    }
                }
            }
            $data = $newData;
        }

        foreach ($data as $k => $v) {
            if (!$this->type->hasProperty($k)) {
                throw new \Exception("Unknown property $k");
            }
        }

        return $this->register(new Entity($data));
    }

    public function __call($method, $arguments)
    {
        foreach ($this->magicMethodRules as $prefix => $oneItem) {
            if (substr($method, 0, strlen($prefix)) == $prefix) {
                $tail = substr($method, strlen($prefix));
                $fields = array_map('strtolower', explode('And', $tail));

                return $this->find(array_combine($fields, $arguments), $oneItem);
            }
        }

        throw new BadMethodCallException("Method $method not found");
    }

    public function findOne($params)
    {
        return $this->find($params, true);

    }

    public function find($params = [], $oneItem = false)
    {
        $query = [];

        if (is_string($params) && 1 * $params == $params) {
            $params = 1 * $params;
        }

        if (is_int($params)) {
            if (isset($this->keyMap[$params])) {
                return $this->entities[$this->keyMap[$params]];
            }
            $query = [
                'id' => $params,
            ];
            $oneItem = true;
        }

        if (is_array($params)) {
            foreach ($params as $key => $value) {

                if (is_numeric($key) && $value instanceof Contracts\Entity) {
                    $key = $this->type->getReferenceProperty($value);
function acceptsInteger($int) { }

$x = '123'; // string "123"

// Instead of
acceptsInteger($x);

// we recommend to use
acceptsInteger((integer) $x);
                }
                if ($this->type->hasProperty($key)) {
                    $query[$key] = $this->type->encodeProperty($key, $value);
                }
            }
        }

        $findKey = md5(json_encode($query));
        if (array_key_exists($findKey, $this->findCache)) {
            return $this->findCache[$findKey];
        }

        $index = $this->type->findIndex(array_keys($query));
        $values = count($query) ? $this->type->getIndexTuple($index, $query) : [];

        $data = $this->type->getSpace()->select($values, $index);

        $result = [];
        if (!empty($data->getData())) {
            foreach ($data->getData() as $tuple) {
                $data = $this->type->fromTuple($tuple);
                if (isset($data['id']) && array_key_exists($data['id'], $this->keyMap)) {
                    $entity = $this->entities[$this->keyMap[$data['id']]];
                    $entity->update($data);
                } else {
                    $entity = new Entity($data);
                    $this->register($entity);
                }
                if ($oneItem) {
                    return $this->findCache[$findKey] = $entity;
                }
                $result[] = $entity;
            }
        }
        if (!$oneItem) {
            return $this->findCache[$findKey] = $result;
class Author {
    private $name;

    public function __construct($name) {
        $this->name = $name;
    }

    public function getName() {
        return $this->name;
    }
}

abstract class Post {
    public function getAuthor() {
        return 'Johannes';
    }
}

class BlogPost extends Post {
    public function getAuthor() {
        return new Author('Johannes');
    }
}

class ForumPost extends Post { /* ... */ }

function my_function(Post $post) {
    echo strtoupper($post->getAuthor());
}
        }
    }

    /**
     * @return Entity
     */
    public function knows(Contracts\Entity $entity)
    {
        return in_array($entity, $this->entities);
    }

    public function remove(Contracts\Entity $entity)
    {
        unset($this->entities[$this->keyMap[$entity->id]]);
        unset($this->keyMap[$entity->id]);

        $this->type->getSpace()->delete([$entity->id]);
interface SomeInterface { }
class SomeClass implements SomeInterface {
    public $a;
}

function someFunction(SomeInterface $object) {
    if ($object instanceof SomeClass) {
        $a = $object->a;
    }
}
    }

    public function save(Contracts\Entity $entity)
    {
        if (!$this->knows($entity)) {
            throw new LogicException('Entity is not related with this repository');
        }

        if (!$entity->getId()) {
            $this->generateId($entity);
            $tuple = $this->type->getTuple($entity->toArray());

            $required = $this->type->getRequiredProperties();

            foreach ($this->type->getProperties() as $index => $field) {
                if (in_array($field, $required) && !array_key_exists($index, $tuple)) {
                    if ($this->type->isReference($field)) {
                        $tuple[$index] = 0;
                    } else {
                        $tuple[$index] = '';
                    }
                }
            }

            // normalize tuple
            if (array_values($tuple) != $tuple) {
                // index was skipped
                $max = max(array_keys($tuple));
                foreach (range(0, $max) as $index) {
                    if (!array_key_exists($index, $tuple)) {
                        $tuple[$index] = null;
                    }
                }
                ksort($tuple);
            }

            $this->type->getSpace()->insert($tuple);
        } else {
            $changes = $entity->pullChanges();
            if (count($changes)) {
                $operations = [];
                foreach ($this->type->getTuple($changes) as $key => $value) {
                    $operations[] = ['=', $key, $value];
                }

                $this->type->getSpace()->update($entity->getId(), $operations);
            }
        }

        return $entity;
    }

    private function register(Contracts\Entity $entity)
    {
        if (!$this->knows($entity)) {
            $this->entities[] = $entity;
        }
        if ($entity->getId() && !array_key_exists($entity->getId(), $this->keyMap)) {
            $this->keyMap[$entity->getId()] = array_search($entity, $this->entities);
        }

        return $entity;
    }

    private function generateId(Contracts\Entity $entity)
    {
        $manager = $this->type->getManager();
        $name = $this->type->getName();
$myVar = 'Value';
$higher = false;

if (rand(1, 6) > 3) {
    $higher = true;
} else {
    $higher = false;
}
        $spaceId = $this->type->getSpaceId();

        $sequence = $manager->get('sequence')->oneBySpace($spaceId);
        if (!$sequence) {
            $sequence = $manager->get('sequence')->create([
                'space' => $spaceId,
                'value' => 0,
            ]);
            $manager->save($sequence);
        }

        $nextValue = +$manager->getMeta()
            ->get('sequence')
            ->getSpace()
            ->update($sequence->id, [['+', 2, 1]])
            ->getData()[0][2];

        $entity->setId($nextValue);

        $this->register($entity);

        return $entity;
    }

    public function getType()
    {
        return $this->type;
    }
}


1			<?php
2
3			namespace Tarantool\Mapper;
4
5			use BadMethodCallException;
6			use LogicException;
7
8			class Repository implements Contracts\Repository
9			{
10			private $type;
11			private $entities = [];
12			private $keyMap = [];
13			private $findCache = [];
14
15			private $magicMethodRules = [
16			'by' => false,
17			'firstBy' => true,
18			'oneBy' => true,
19			];
20
21	26		public function __construct(Contracts\Type $type)
22			{
23	26		$this->type = $type;
24	26		}
25
26	26		public function create($data = null)
27			{
28	26		if ($data && !is_array($data)) {
29	26		$properties = $this->getType()->getProperties();
30	26		if (count($properties) == 2) {
31	26		$data = [$properties[1] => $data];
32			} else {
33	1		throw new LogicException('Data should be array');
34			}
35			}
36	26		if ($data) {
37	26		$newData = [];
38	26	View Code Duplication	foreach ($data as $k => $v) {
			0 ignored issues – show Duplication introduced 2016-04-10 11:30 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...
39	26		if (!is_numeric($k)) {
40	26		$newData[$k] = $v;
41			} else {
42	4		if ($v instanceof Contracts\Entity) {
43	3		$type = $this->type->getManager()->findRepository($v)->getType();
44	26		$newData[$this->type->getReferenceProperty($type)] = $v;
45			}
46			}
47			}
48	26		$data = $newData;
49			}
50
51	26		foreach ($data as $k => $v) {
52	26		if (!$this->type->hasProperty($k)) {
53	26		throw new \Exception("Unknown property $k");
54			}
55			}
56
57	26		return $this->register(new Entity($data));
58			}
59
60	26		public function __call($method, $arguments)
61			{
62	26		foreach ($this->magicMethodRules as $prefix => $oneItem) {
63	26		if (substr($method, 0, strlen($prefix)) == $prefix) {
64	26		$tail = substr($method, strlen($prefix));
65	26		$fields = array_map('strtolower', explode('And', $tail));
66
67	26		return $this->find(array_combine($fields, $arguments), $oneItem);
68			}
69			}
70
71	1		throw new BadMethodCallException("Method $method not found");
72			}
73
74	5		public function findOne($params)
75			{
76	5		return $this->find($params, true);
			0 ignored issues – show Bug Compatibility introduced 2016-04-04 18:16 UTC by Report Bug Copy Issue Report The expression `$this->find($params, true);` of type `Tarantool\Mapper\Contrac...pper\Contracts\Entity[]` adds the type `Tarantool\Mapper\Contracts\Entity[]` to the return on line 76 which is incompatible with the return type declared by the interface `Tarantool\Mapper\Contracts\Repository::findOne` of type `Tarantool\Mapper\Contracts\Entity`. Loading history...
77			}
78
79	26		public function find($params = [], $oneItem = false)
80			{
81	26		$query = [];
82
83	26		if (is_string($params) && 1 * $params == $params) {
84	1		$params = 1 * $params;
85			}
86
87	26		if (is_int($params)) {
88	5		if (isset($this->keyMap[$params])) {
89	1		return $this->entities[$this->keyMap[$params]];
90			}
91			$query = [
92	4		'id' => $params,
93			];
94	4		$oneItem = true;
95			}
96
97	26		if (is_array($params)) {
98	26	View Code Duplication	foreach ($params as $key => $value) {
			0 ignored issues – show Duplication introduced 2016-04-10 11:30 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...
99	26		if (is_numeric($key) && $value instanceof Contracts\Entity) {
100			$key = $this->type->getReferenceProperty($value);
			0 ignored issues – show Documentation introduced 2016-04-10 11:30 UTC by Report Bug Copy Issue Report `$value` is of type `object<Tarantool\Mapper\Contracts\Entity>`, but the function expects a `object<Tarantool\Mapper\Contracts\Type>`. 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...
101			}
102	26		if ($this->type->hasProperty($key)) {
103	26		$query[$key] = $this->type->encodeProperty($key, $value);
104			}
105			}
106			}
107
108	26		$findKey = md5(json_encode($query));
109	26		if (array_key_exists($findKey, $this->findCache)) {
110	26		return $this->findCache[$findKey];
111			}
112
113	26		$index = $this->type->findIndex(array_keys($query));
114	26		$values = count($query) ? $this->type->getIndexTuple($index, $query) : [];
115
116	26		$data = $this->type->getSpace()->select($values, $index);
117
118	26		$result = [];
119	26		if (!empty($data->getData())) {
120	26		foreach ($data->getData() as $tuple) {
121	26		$data = $this->type->fromTuple($tuple);
122	26		if (isset($data['id']) && array_key_exists($data['id'], $this->keyMap)) {
123	26		$entity = $this->entities[$this->keyMap[$data['id']]];
124	26		$entity->update($data);
125			} else {
126	26		$entity = new Entity($data);
127	26		$this->register($entity);
128			}
129	26		if ($oneItem) {
130	26		return $this->findCache[$findKey] = $entity;
131			}
132	5		$result[] = $entity;
133			}
134			}
135	26		if (!$oneItem) {
136	5		return $this->findCache[$findKey] = $result;
			0 ignored issues – show Bug Best Practice introduced 2016-04-07 17:41 UTC by Report Bug Copy Issue Report The return type of `return $this->findCache[$findKey] = $result;` (`array`) is incompatible with the return type declared by the interface `Tarantool\Mapper\Contracts\Repository::find` of type `Tarantool\Mapper\Contrac...pper\Contracts\Entity[]`. If you return a value from a function or method, it should be a sub-type of the type that is given by the parent type f.e. an interface, or abstract method. This is more formally defined by the Lizkov substitution principle, and guarantees that classes that depend on the parent type can use any instance of a child type interchangably. This principle also belongs to the SOLID principles for object oriented design. Let’s take a look at an example: class Author { private $name; public function __construct($name) { $this->name = $name; } public function getName() { return $this->name; } } abstract class Post { public function getAuthor() { return 'Johannes'; } } class BlogPost extends Post { public function getAuthor() { return new Author('Johannes'); } } class ForumPost extends Post { /* ... */ } function my_function(Post $post) { echo strtoupper($post->getAuthor()); } Our function `my_function` expects a `Post` object, and outputs the author of the post. The base class `Post` returns a simple string and outputting a simple string will work just fine. However, the child class `BlogPost` which is a sub-type of `Post` instead decided to return an `object`, and is therefore violating the SOLID principles. If a `BlogPost` were passed to `my_function`, PHP would not complain, but ultimately fail when executing the `strtoupper` call in its body. Loading history...
137			}
138	26		}
139
140			/**
141			* @return Entity
142			*/
143	26		public function knows(Contracts\Entity $entity)
144			{
145	26		return in_array($entity, $this->entities);
146			}
147
148	1		public function remove(Contracts\Entity $entity)
149			{
150	1		unset($this->entities[$this->keyMap[$entity->id]]);
151	1		unset($this->keyMap[$entity->id]);
152
153	1		$this->type->getSpace()->delete([$entity->id]);
			0 ignored issues – show Bug introduced 2016-04-04 18:16 UTC by Report Bug Copy Issue Report Accessing `id` on the interface `Tarantool\Mapper\Contracts\Entity` suggest that you code against a concrete implementation. How about adding an `instanceof` check? If you access a property on an interface, you most likely code against a concrete implementation of the interface. Available Fixes Adding an additional type check: interface SomeInterface { } class SomeClass implements SomeInterface { public $a; } function someFunction(SomeInterface $object) { if ($object instanceof SomeClass) { $a = $object->a; } } Changing the type hint: interface SomeInterface { } class SomeClass implements SomeInterface { public $a; } function someFunction(SomeClass $object) { $a = $object->a; } Loading history...
154	1		}
155
156	26		public function save(Contracts\Entity $entity)
157			{
158	26		if (!$this->knows($entity)) {
159	1		throw new LogicException('Entity is not related with this repository');
160			}
161
162	26		if (!$entity->getId()) {
163	26		$this->generateId($entity);
164	26		$tuple = $this->type->getTuple($entity->toArray());
165
166	26		$required = $this->type->getRequiredProperties();
167
168	26		foreach ($this->type->getProperties() as $index => $field) {
169	26		if (in_array($field, $required) && !array_key_exists($index, $tuple)) {
170	1		if ($this->type->isReference($field)) {
171	1		$tuple[$index] = 0;
172			} else {
173	26		$tuple[$index] = '';
174			}
175			}
176			}
177
178			// normalize tuple
179	26		if (array_values($tuple) != $tuple) {
180			// index was skipped
181	1		$max = max(array_keys($tuple));
182	1		foreach (range(0, $max) as $index) {
183	1		if (!array_key_exists($index, $tuple)) {
184	1		$tuple[$index] = null;
185			}
186			}
187	1		ksort($tuple);
188			}
189
190	26		$this->type->getSpace()->insert($tuple);
191			} else {
192	5		$changes = $entity->pullChanges();
193	5		if (count($changes)) {
194	5		$operations = [];
195	5		foreach ($this->type->getTuple($changes) as $key => $value) {
196	5		$operations[] = ['=', $key, $value];
197			}
198
199	5		$this->type->getSpace()->update($entity->getId(), $operations);
200			}
201			}
202
203	26		return $entity;
204			}
205
206	26		private function register(Contracts\Entity $entity)
207			{
208	26		if (!$this->knows($entity)) {
209	26		$this->entities[] = $entity;
210			}
211	26		if ($entity->getId() && !array_key_exists($entity->getId(), $this->keyMap)) {
212	26		$this->keyMap[$entity->getId()] = array_search($entity, $this->entities);
213			}
214
215	26		return $entity;
216			}
217
218	26		private function generateId(Contracts\Entity $entity)
219			{
220	26		$manager = $this->type->getManager();
221	26		$name = $this->type->getName();
			0 ignored issues – show Unused Code introduced 2016-04-04 18:16 UTC by Report Bug Copy Issue Report `$name` is not used, you could remove the assignment. This check looks for variable assignements that are either overwritten by other assignments or where the variable is not used subsequently. $myVar = 'Value'; $higher = false; if (rand(1, 6) > 3) { $higher = true; } else { $higher = false; } Both the `$myVar` assignment in line 1 and the `$higher` assignment in line 2 are dead. The first because `$myVar` is never used and the second because `$higher` is always overwritten for every possible time line. Loading history...
222	26		$spaceId = $this->type->getSpaceId();
223
224	26		$sequence = $manager->get('sequence')->oneBySpace($spaceId);
225	26		if (!$sequence) {
226	26		$sequence = $manager->get('sequence')->create([
227	26		'space' => $spaceId,
228	26		'value' => 0,
229			]);
230	26		$manager->save($sequence);
231			}
232
233	26		$nextValue = +$manager->getMeta()
234	26		->get('sequence')
235	26		->getSpace()
236	26		->update($sequence->id, [['+', 2, 1]])
237	26		->getData()[0][2];
238
239	26		$entity->setId($nextValue);
240
241	26		$this->register($entity);
242
243	26		return $entity;
244			}
245
246	26		public function getType()
247			{
248	26		return $this->type;
249			}
250			}
251

tarantool-php / mapper