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:
1 | <?php |
||
34 | class DataOperations { |
||
35 | |||
36 | private $wrapper; |
||
37 | |||
38 | /** |
||
39 | * |
||
40 | * @var DriverAdapter |
||
41 | */ |
||
42 | private $adapter; |
||
43 | private $data; |
||
44 | private $invalidFields; |
||
45 | private $hasMultipleData; |
||
46 | private $driver; |
||
47 | private $container; |
||
48 | |||
49 | const MODE_SAVE = 0; |
||
50 | const MODE_UPDATE = 1; |
||
51 | |||
52 | 35 | View Code Duplication | public function __construct(ORMContext $context, RecordWrapper $wrapper, DriverAdapter $adapter) { |
58 | |||
59 | 10 | public function doSave($hasMultipleData) { |
|
60 | 10 | $this->hasMultipleData = $hasMultipleData; |
|
61 | 10 | $invalidFields = []; |
|
62 | 10 | $data = $this->wrapper->getData(); |
|
63 | 10 | $primaryKey = $this->wrapper->getDescription()->getPrimaryKey(); |
|
64 | 10 | $singlePrimaryKey = null; |
|
|
|||
65 | 10 | $succesful = true; |
|
66 | |||
67 | 10 | if (count($primaryKey) == 1) { |
|
68 | 10 | $singlePrimaryKey = $primaryKey[0]; |
|
69 | } |
||
70 | |||
71 | // Assign an empty array to force a validation error for empty models |
||
72 | 10 | if (empty($data)) { |
|
73 | 2 | $data = [[]]; |
|
74 | } |
||
75 | |||
76 | 10 | $this->driver->beginTransaction(); |
|
77 | |||
78 | 10 | foreach ($data as $i => $datum) { |
|
79 | 10 | $status = $this->saveRecord($datum, $primaryKey); |
|
80 | 6 | $data[$i] = $datum; |
|
81 | |||
82 | 6 | if (!$status['success']) { |
|
83 | $succesful = false; |
||
84 | $invalidFields[$i] = $status['invalid_fields']; |
||
85 | $this->driver->rollback(); |
||
86 | 6 | break; |
|
87 | } |
||
88 | } |
||
89 | |||
90 | 6 | if ($succesful) { |
|
91 | 6 | $this->driver->commit(); |
|
92 | } else { |
||
93 | $this->assignValue($this->invalidFields, $invalidFields); |
||
94 | } |
||
95 | |||
96 | 6 | $this->wrapper->setData($hasMultipleData ? $data : $data[0]); |
|
97 | |||
98 | 6 | return $succesful; |
|
99 | } |
||
100 | |||
101 | public function doValidate() { |
||
109 | |||
110 | /** |
||
111 | * Save an individual record. |
||
112 | * |
||
113 | * @param array $record The record to be saved |
||
114 | * @param type $primaryKey The primary keys of the record |
||
115 | * @return boolean |
||
116 | */ |
||
117 | 10 | private function saveRecord(&$record, $primaryKey) { |
|
186 | |||
187 | 10 | private function validate($data, $mode) { |
|
199 | |||
200 | 10 | private function isPrimaryKeySet($primaryKey, $data) { |
|
211 | |||
212 | private function assignValue(&$property, $value) { |
||
219 | |||
220 | public function getData() { |
||
223 | |||
224 | 10 | public function getInvalidFields() { |
|
227 | |||
228 | public function isItemDeletable($primaryKey, $data) { |
||
235 | |||
236 | } |
||
237 |
This check looks for variable assignements that are either overwritten by other assignments or where the variable is not used subsequently.
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.