Conditions | 10 |
Paths | 10 |
Total Lines | 69 |
Code Lines | 45 |
Lines | 0 |
Ratio | 0 % |
Changes | 11 | ||
Bugs | 3 | Features | 0 |
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:
If many parameters/temporary variables are present:
1 | <?php |
||
124 | public function onParentSave(AbstractEntityEvent $event) |
||
125 | { |
||
126 | $connection = $event->getConnection(); |
||
127 | $parentRegistry = $event->getTable()->getRegistry(); |
||
128 | $table = $connection->table($this->getTableName()); |
||
129 | $registry = $table->getRegistry(); |
||
130 | |||
131 | foreach ($this->getWorkersQueue() as $worker) { |
||
132 | $worker->setRegistry($registry); |
||
133 | $entity = $worker->getEntity(); |
||
134 | $access = new Accessor($this->parent); |
||
135 | |||
136 | $entry = $parentRegistry->getEntry($this->parent); |
||
137 | $ids = $entry->getIdentifiers(); |
||
138 | |||
139 | if (!count($ids)) { |
||
140 | throw new \RuntimeException (sprintf('Parent (%s) have no identifiers defined', get_class($this->parent))); |
||
141 | } |
||
142 | |||
143 | if (!array_key_exists($this->local, $ids)) { |
||
144 | throw new \RuntimeException (sprintf('Local key "%s" is not a valid identifier (primary key on %s)', $this->local, $registry->getTableName())); |
||
145 | } |
||
146 | |||
147 | if ($worker instanceof DeleteEntityWorker) { |
||
148 | $value = $access->get($this->local); |
||
149 | if (empty($value)) { |
||
150 | throw new \RuntimeException (sprintf('Identifier not set on %s (column: %s)', get_class($this->parent), $this->local)); |
||
151 | } |
||
152 | |||
153 | $params[] = $value; |
||
|
|||
154 | $acc = Accessor::factory($entity)->get($this->local); |
||
155 | $params[] = $acc; |
||
156 | |||
157 | if (empty($acc)) { |
||
158 | throw new \RuntimeException (sprintf('Identifier not set on %s (column: %s)', get_class($entity), $this->local)); |
||
159 | } |
||
160 | |||
161 | $connection->execute( |
||
162 | Query::factory() |
||
163 | ->delete($this->foreignTable) |
||
164 | ->where(sprintf('%s = ?', $this->foreign)) |
||
165 | ->andWhere(sprintf('%s = ?', $this->foreignLink)), |
||
166 | $params |
||
167 | ); |
||
168 | |||
169 | $this->getRegistry()->remove($entity); |
||
170 | continue; |
||
171 | } |
||
172 | |||
173 | if ($worker instanceof SaveEntityWorker) { |
||
174 | $value = $access->get($this->local); |
||
175 | Accessor::factory($entity)->set($this->foreign, $value); |
||
176 | $registry->markForAction($entity, Registry::ACTION_SAVE); |
||
177 | } |
||
178 | |||
179 | $worker->execute($connection); |
||
180 | |||
181 | if ($worker instanceof SaveEntityWorker && $this->getRegistry()->getState($entity) == RegistryState::REGISTERED) { |
||
182 | $connection->execute( |
||
183 | Query::factory() |
||
184 | ->insert($this->foreignTable) |
||
185 | ->set($this->foreign, '?') |
||
186 | ->set($this->foreignLink, '?'), |
||
187 | array($access->get($this->local), Accessor::factory($entity)->get($this->foreignRefs)) |
||
188 | ); |
||
189 | $this->getRegistry()->defineInitialValues($entity, $connection, $table); |
||
190 | } |
||
191 | } |
||
192 | } |
||
193 | |||
251 | } |
Adding an explicit array definition is generally preferable to implicit array definition as it guarantees a stable state of the code.
Let’s take a look at an example:
As you can see in this example, the array
$myArray
is initialized the first time when the foreach loop is entered. You can also see that the value of thebar
key is only written conditionally; thus, its value might result from a previous iteration.This might or might not be intended. To make your intention clear, your code more readible and to avoid accidental bugs, we recommend to add an explicit initialization $myArray = array() either outside or inside the foreach loop.