Completed
Push — master ( e4e252...9e06e2 )
by Dmitry
02:48
created

Repository::find()   C

Complexity

Conditions 18
Paths 106

Size

Total Lines 60
Code Lines 37

Duplication

Lines 8
Ratio 13.33 %

Code Coverage

Tests 35
CRAP Score 18.0069

Importance

Changes 16
Bugs 3 Features 6
Metric Value
c 16
b 3
f 6
dl 8
loc 60
ccs 35
cts 36
cp 0.9722
rs 6.062
cc 18
eloc 37
nc 106
nop 2
crap 18.0069

How to fix   Long Method    Complexity   

Long Method

Small methods make your code easier to understand, in particular if combined with a good name. Besides, if your method is small, finding a good name is usually much easier.

For example, if you find yourself adding comments to a method's body, this is usually a good sign to extract the commented part to a new method, and use the comment as a starting point when coming up with a good name for this new method.

Commonly applied refactorings include:

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 by
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 by
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 by
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 by
$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 by
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 by
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

  1. Adding an additional type check:

    interface SomeInterface { }
    class SomeClass implements SomeInterface {
        public $a;
    }
    
    function someFunction(SomeInterface $object) {
        if ($object instanceof SomeClass) {
            $a = $object->a;
        }
    }
    
  2. 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 by
$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