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 Cell 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 Cell, and based on these observations, apply Extract Interface, too.
1 | <?php |
||
13 | class Cell |
||
14 | { |
||
15 | /** |
||
16 | * Value binder to use. |
||
17 | * |
||
18 | * @var IValueBinder |
||
19 | */ |
||
20 | private static $valueBinder; |
||
21 | |||
22 | /** |
||
23 | * Value of the cell. |
||
24 | * |
||
25 | * @var mixed |
||
26 | */ |
||
27 | private $value; |
||
28 | |||
29 | /** |
||
30 | * Calculated value of the cell (used for caching) |
||
31 | * This returns the value last calculated by MS Excel or whichever spreadsheet program was used to |
||
32 | * create the original spreadsheet file. |
||
33 | * Note that this value is not guaranteed to reflect the actual calculated value because it is |
||
34 | * possible that auto-calculation was disabled in the original spreadsheet, and underlying data |
||
35 | * values used by the formula have changed since it was last calculated. |
||
36 | * |
||
37 | * @var mixed |
||
38 | */ |
||
39 | private $calculatedValue; |
||
40 | |||
41 | /** |
||
42 | * Type of the cell data. |
||
43 | * |
||
44 | * @var string |
||
45 | */ |
||
46 | private $dataType; |
||
47 | |||
48 | /** |
||
49 | * Collection of cells. |
||
50 | * |
||
51 | * @var Cells |
||
52 | */ |
||
53 | private $parent; |
||
54 | |||
55 | /** |
||
56 | * Index to cellXf. |
||
57 | * |
||
58 | * @var int |
||
59 | */ |
||
60 | private $xfIndex = 0; |
||
61 | |||
62 | /** |
||
63 | * Attributes of the formula. |
||
64 | */ |
||
65 | private $formulaAttributes; |
||
66 | |||
67 | /** |
||
68 | * Update the cell into the cell collection. |
||
69 | * |
||
70 | * @return self |
||
71 | */ |
||
72 | 121 | public function updateInCollection() |
|
78 | |||
79 | 111 | public function detach() |
|
83 | |||
84 | 103 | public function attach(Cells $parent) |
|
88 | |||
89 | /** |
||
90 | * Create a new Cell. |
||
91 | * |
||
92 | * @param mixed $pValue |
||
93 | * @param string $pDataType |
||
94 | * @param Worksheet $pSheet |
||
95 | * |
||
96 | * @throws Exception |
||
97 | */ |
||
98 | 125 | public function __construct($pValue, $pDataType, Worksheet $pSheet) |
|
116 | |||
117 | /** |
||
118 | * Get cell coordinate column. |
||
119 | * |
||
120 | * @return string |
||
121 | */ |
||
122 | 48 | public function getColumn() |
|
126 | |||
127 | /** |
||
128 | * Get cell coordinate row. |
||
129 | * |
||
130 | * @return int |
||
131 | */ |
||
132 | 46 | public function getRow() |
|
136 | |||
137 | /** |
||
138 | * Get cell coordinate. |
||
139 | * |
||
140 | * @return string |
||
141 | */ |
||
142 | 125 | public function getCoordinate() |
|
146 | |||
147 | /** |
||
148 | * Get cell value. |
||
149 | * |
||
150 | * @return mixed |
||
151 | */ |
||
152 | 109 | public function getValue() |
|
156 | |||
157 | /** |
||
158 | * Get cell value with formatting. |
||
159 | * |
||
160 | * @return string |
||
161 | */ |
||
162 | 5 | public function getFormattedValue() |
|
170 | |||
171 | /** |
||
172 | * Set cell value. |
||
173 | * |
||
174 | * Sets the value for a cell, automatically determining the datatype using the value binder |
||
175 | * |
||
176 | * @param mixed $pValue Value |
||
177 | * |
||
178 | * @throws Exception |
||
179 | * |
||
180 | * @return Cell |
||
181 | */ |
||
182 | 79 | public function setValue($pValue) |
|
190 | |||
191 | /** |
||
192 | * Set the value for a cell, with the explicit data type passed to the method (bypassing any use of the value binder). |
||
193 | * |
||
194 | * @param mixed $pValue Value |
||
195 | * @param string $pDataType Explicit data type, see DataType::TYPE_* |
||
196 | * |
||
197 | * @throws Exception |
||
198 | * |
||
199 | * @return Cell |
||
200 | */ |
||
201 | 117 | public function setValueExplicit($pValue, $pDataType) |
|
245 | |||
246 | /** |
||
247 | * Get calculated cell value. |
||
248 | * |
||
249 | * @param bool $resetLog Whether the calculation engine logger should be reset or not |
||
250 | * |
||
251 | * @throws Exception |
||
252 | * |
||
253 | * @return mixed |
||
254 | */ |
||
255 | 71 | public function getCalculatedValue($resetLog = true) |
|
289 | |||
290 | /** |
||
291 | * Set old calculated value (cached). |
||
292 | * |
||
293 | * @param mixed $pValue Value |
||
294 | * |
||
295 | * @return Cell |
||
296 | */ |
||
297 | 22 | public function setCalculatedValue($pValue) |
|
305 | |||
306 | /** |
||
307 | * Get old calculated value (cached) |
||
308 | * This returns the value last calculated by MS Excel or whichever spreadsheet program was used to |
||
309 | * create the original spreadsheet file. |
||
310 | * Note that this value is not guaranteed to refelect the actual calculated value because it is |
||
311 | * possible that auto-calculation was disabled in the original spreadsheet, and underlying data |
||
312 | * values used by the formula have changed since it was last calculated. |
||
313 | * |
||
314 | * @return mixed |
||
315 | */ |
||
316 | public function getOldCalculatedValue() |
||
320 | |||
321 | /** |
||
322 | * Get cell data type. |
||
323 | * |
||
324 | * @return string |
||
325 | */ |
||
326 | 70 | public function getDataType() |
|
330 | |||
331 | /** |
||
332 | * Set cell data type. |
||
333 | * |
||
334 | * @param string $pDataType see DataType::TYPE_* |
||
335 | * |
||
336 | * @return Cell |
||
337 | */ |
||
338 | public function setDataType($pDataType) |
||
347 | |||
348 | /** |
||
349 | * Identify if the cell contains a formula. |
||
350 | * |
||
351 | * @return bool |
||
352 | */ |
||
353 | public function isFormula() |
||
357 | |||
358 | /** |
||
359 | * Does this cell contain Data validation rules? |
||
360 | * |
||
361 | * @throws Exception |
||
362 | * |
||
363 | * @return bool |
||
364 | */ |
||
365 | 3 | public function hasDataValidation() |
|
373 | |||
374 | /** |
||
375 | * Get Data validation rules. |
||
376 | * |
||
377 | * @throws Exception |
||
378 | * |
||
379 | * @return DataValidation |
||
380 | */ |
||
381 | 4 | public function getDataValidation() |
|
389 | |||
390 | /** |
||
391 | * Set Data validation rules. |
||
392 | * |
||
393 | * @param DataValidation $pDataValidation |
||
394 | * |
||
395 | * @throws Exception |
||
396 | * |
||
397 | * @return Cell |
||
398 | */ |
||
399 | View Code Duplication | public function setDataValidation(DataValidation $pDataValidation = null) |
|
409 | |||
410 | /** |
||
411 | * Does this cell contain valid value? |
||
412 | * |
||
413 | * @return bool |
||
414 | */ |
||
415 | 3 | public function hasValidValue() |
|
421 | |||
422 | /** |
||
423 | * Does this cell contain a Hyperlink? |
||
424 | * |
||
425 | * @throws Exception |
||
426 | * |
||
427 | * @return bool |
||
428 | */ |
||
429 | public function hasHyperlink() |
||
437 | |||
438 | /** |
||
439 | * Get Hyperlink. |
||
440 | * |
||
441 | * @throws Exception |
||
442 | * |
||
443 | * @return Hyperlink |
||
444 | */ |
||
445 | 21 | public function getHyperlink() |
|
453 | |||
454 | /** |
||
455 | * Set Hyperlink. |
||
456 | * |
||
457 | * @param Hyperlink $pHyperlink |
||
458 | * |
||
459 | * @throws Exception |
||
460 | * |
||
461 | * @return Cell |
||
462 | */ |
||
463 | View Code Duplication | public function setHyperlink(Hyperlink $pHyperlink = null) |
|
473 | |||
474 | /** |
||
475 | * Get cell collection. |
||
476 | * |
||
477 | * @return Cells |
||
478 | */ |
||
479 | 64 | public function getParent() |
|
483 | |||
484 | /** |
||
485 | * Get parent worksheet. |
||
486 | * |
||
487 | * @return Worksheet |
||
488 | */ |
||
489 | 80 | public function getWorksheet() |
|
493 | |||
494 | /** |
||
495 | * Is this cell in a merge range. |
||
496 | * |
||
497 | * @return bool |
||
498 | */ |
||
499 | public function isInMergeRange() |
||
503 | |||
504 | /** |
||
505 | * Is this cell the master (top left cell) in a merge range (that holds the actual data value). |
||
506 | * |
||
507 | * @return bool |
||
508 | */ |
||
509 | public function isMergeRangeValueCell() |
||
521 | |||
522 | /** |
||
523 | * If this cell is in a merge range, then return the range. |
||
524 | * |
||
525 | * @return string |
||
526 | */ |
||
527 | public function getMergeRange() |
||
537 | |||
538 | /** |
||
539 | * Get cell style. |
||
540 | * |
||
541 | * @return Style |
||
542 | */ |
||
543 | 6 | public function getStyle() |
|
547 | |||
548 | /** |
||
549 | * Re-bind parent. |
||
550 | * |
||
551 | * @param Worksheet $parent |
||
552 | * |
||
553 | * @return Cell |
||
554 | */ |
||
555 | public function rebindParent(Worksheet $parent) |
||
561 | |||
562 | /** |
||
563 | * Is cell in a specific range? |
||
564 | * |
||
565 | * @param string $pRange Cell range (e.g. A1:A1) |
||
566 | * |
||
567 | * @return bool |
||
568 | */ |
||
569 | public function isInRange($pRange) |
||
581 | |||
582 | /** |
||
583 | * Compare 2 cells. |
||
584 | * |
||
585 | * @param Cell $a Cell a |
||
586 | * @param Cell $b Cell b |
||
587 | * |
||
588 | * @return int Result of comparison (always -1 or 1, never zero!) |
||
589 | */ |
||
590 | public static function compareCells(Cell $a, Cell $b) |
||
602 | |||
603 | /** |
||
604 | * Get value binder to use. |
||
605 | * |
||
606 | * @return IValueBinder |
||
607 | */ |
||
608 | 79 | public static function getValueBinder() |
|
616 | |||
617 | /** |
||
618 | * Set value binder to use. |
||
619 | * |
||
620 | * @param IValueBinder $binder |
||
621 | * |
||
622 | * @throws Exception |
||
623 | */ |
||
624 | 2 | public static function setValueBinder(IValueBinder $binder) |
|
628 | |||
629 | /** |
||
630 | * Implement PHP __clone to create a deep clone, not just a shallow copy. |
||
631 | */ |
||
632 | 2 | View Code Duplication | public function __clone() |
643 | |||
644 | /** |
||
645 | * Get index to cellXf. |
||
646 | * |
||
647 | * @return int |
||
648 | */ |
||
649 | 99 | public function getXfIndex() |
|
653 | |||
654 | /** |
||
655 | * Set index to cellXf. |
||
656 | * |
||
657 | * @param int $pValue |
||
658 | * |
||
659 | * @return Cell |
||
660 | */ |
||
661 | 92 | public function setXfIndex($pValue) |
|
667 | |||
668 | /** |
||
669 | * Set the formula attributes. |
||
670 | * |
||
671 | * @param mixed $pAttributes |
||
672 | */ |
||
673 | public function setFormulaAttributes($pAttributes) |
||
679 | |||
680 | /** |
||
681 | * Get the formula attributes. |
||
682 | */ |
||
683 | 18 | public function getFormulaAttributes() |
|
687 | |||
688 | /** |
||
689 | * Convert to string. |
||
690 | * |
||
691 | * @return string |
||
692 | */ |
||
693 | public function __toString() |
||
697 | } |
||
698 |
This check looks for unreachable code. It uses sophisticated control flow analysis techniques to find statements which will never be executed.
Unreachable code is most often the result of
return
,die
orexit
statements that have been added for debug purposes.In the above example, the last
return false
will never be executed, because a return statement has already been met in every possible execution path.