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:
Complex classes like AbstractFactory 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 AbstractFactory, and based on these observations, apply Extract Interface, too.
1 | <?php |
||
15 | abstract class AbstractFactory implements FactoryInterface |
||
16 | { |
||
17 | /** |
||
18 | * @var array $resolved |
||
19 | */ |
||
20 | static protected $resolved = []; |
||
21 | |||
22 | /** |
||
23 | * If a base class is set, then it must be ensured that the |
||
24 | * @var string $baseClass |
||
25 | */ |
||
26 | private $baseClass = ''; |
||
27 | /** |
||
28 | * |
||
29 | * @var string $defaultClass |
||
30 | */ |
||
31 | private $defaultClass = ''; |
||
32 | |||
33 | /** |
||
34 | * @var array $arguments |
||
35 | */ |
||
36 | private $arguments; |
||
37 | |||
38 | /** |
||
39 | * @var callable $callback |
||
40 | */ |
||
41 | private $callback; |
||
42 | |||
43 | /** |
||
44 | * Keeps loaded instances in memory, in `[$type => $instance]` format. |
||
45 | * Used with the `get()` method only. |
||
46 | * @var array $instances |
||
47 | */ |
||
48 | private $instances = []; |
||
49 | |||
50 | /** |
||
51 | * @param array $data Constructor dependencies. |
||
52 | */ |
||
53 | public function __construct(array $data=null) |
||
68 | |||
69 | /** |
||
70 | * Create a new instance of a class, by type. |
||
71 | * |
||
72 | * Unlike `get()`, this method *always* return a new instance of the requested class. |
||
73 | * |
||
74 | * ## Object callback |
||
75 | * It is possible to pass a callback method that will be executed upon object instanciation. |
||
76 | * The callable should have a signature: `function($obj);` where $obj is the newly created object. |
||
77 | * |
||
78 | * |
||
79 | * @param string $type The type (class ident). |
||
80 | * @param array $args Optional. The constructor arguments. Leave blank to use `$arguments` member. |
||
81 | * @param callable $cb Optional. Object callback, called at creation. Leave blank to use `$callback` member. |
||
82 | * @throws Exception If the base class is set and the resulting instance is not of the base class. |
||
83 | * @throws InvalidArgumentException If type argument is not a string or is not an available type. |
||
84 | * @return mixed The instance / object |
||
85 | */ |
||
86 | final public function create($type, array $args = null, callable $cb = null) |
||
155 | |||
156 | /** |
||
157 | * Create a class instance with given arguments. |
||
158 | * |
||
159 | * How the constructor arguments are passed depends on its type: |
||
160 | * |
||
161 | * - if null, no arguments are passed at all. |
||
162 | * - if it's not an array, it's passed as a single argument. |
||
163 | * - if it's an associative array, it's passed as a sing argument. |
||
164 | * - if it's a sequential (numeric keys) array, it's |
||
165 | */ |
||
166 | public function createClass($classname, $args) |
||
187 | |||
188 | /** |
||
189 | * Get (load or create) an instance of a class, by type. |
||
190 | * |
||
191 | * Unlike `create()` (which always call a `new` instance), this function first tries to load / reuse |
||
192 | * an already created object of this type, from memory. |
||
193 | * |
||
194 | * @param string $type The type (class ident). |
||
195 | * @param array $args The constructor arguments (optional). |
||
196 | * @throws InvalidArgumentException If type argument is not a string. |
||
197 | * @return mixed The instance / object |
||
198 | */ |
||
199 | final public function get($type, array $args = null) |
||
211 | |||
212 | /** |
||
213 | * If a base class is set, then it must be ensured that the created objects |
||
214 | * are `instanceof` this base class. |
||
215 | * |
||
216 | * @param string $type The FQN of the class, or "type" of object, to set as base class. |
||
217 | * @throws InvalidArgumentException If the class is not a string or is not an existing class / interface. |
||
218 | * @return FactoryInterface |
||
219 | */ |
||
220 | public function setBaseClass($type) |
||
246 | |||
247 | /** |
||
248 | * @return string The FQN of the base class |
||
249 | */ |
||
250 | public function baseClass() |
||
254 | |||
255 | /** |
||
256 | * If a default class is set, then calling `get()` or `create()` an invalid type |
||
257 | * should return an object of this class instead of throwing an error. |
||
258 | * |
||
259 | * @param string $type The FQN of the class, or "type" of object, to set as default class. |
||
260 | * @throws InvalidArgumentException If the class name is not a string or not a valid class. |
||
261 | * @return FactoryInterface |
||
262 | */ |
||
263 | public function setDefaultClass($type) |
||
287 | |||
288 | /** |
||
289 | * @return string The FQN of the default class |
||
290 | */ |
||
291 | public function defaultClass() |
||
295 | |||
296 | /** |
||
297 | * @param array $arguments The constructor arguments to be passed to the created object's initialization. |
||
298 | * @return AbstractFactory Chainable |
||
299 | */ |
||
300 | public function setArguments(array $arguments) |
||
305 | |||
306 | /** |
||
307 | * @return array |
||
308 | */ |
||
309 | public function arguments() |
||
313 | |||
314 | /** |
||
315 | * @param callable $callback The object callback. |
||
316 | * @return AbstractFatory Chainable |
||
317 | */ |
||
318 | public function setCallback(callable $callback) |
||
323 | |||
324 | /** |
||
325 | * @return callable|null |
||
326 | */ |
||
327 | public function callback() |
||
331 | |||
332 | /** |
||
333 | * Resolve the class name from "type". |
||
334 | * |
||
335 | * @param string $type The "type" of object to resolve (the object ident). |
||
336 | * @return string The resolved class name (FQN). |
||
337 | */ |
||
338 | abstract public function resolve($type); |
||
339 | |||
340 | /** |
||
341 | * Returns wether a type is resolvable (is valid). |
||
342 | * |
||
343 | * @param string $type The "type" of object to resolve (the object ident). |
||
344 | * @return boolean True if the type is available, false if not |
||
345 | */ |
||
346 | abstract public function isResolvable($type); |
||
347 | } |
||
348 |
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.