| 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
$myArrayis initialized the first time when the foreach loop is entered. You can also see that the value of thebarkey 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.