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 Spreadsheet 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 Spreadsheet, and based on these observations, apply Extract Interface, too.
1 | <?php |
||
29 | class Spreadsheet |
||
30 | { |
||
31 | /** |
||
32 | * Unique ID |
||
33 | * |
||
34 | * @var string |
||
35 | */ |
||
36 | private $uniqueID; |
||
37 | |||
38 | /** |
||
39 | * Document properties |
||
40 | * |
||
41 | * @var Document\Properties |
||
42 | */ |
||
43 | private $properties; |
||
44 | |||
45 | /** |
||
46 | * Document security |
||
47 | * |
||
48 | * @var Document\Security |
||
49 | */ |
||
50 | private $security; |
||
51 | |||
52 | /** |
||
53 | * Collection of Worksheet objects |
||
54 | * |
||
55 | * @var Worksheet[] |
||
56 | */ |
||
57 | private $workSheetCollection = array(); |
||
58 | |||
59 | /** |
||
60 | * Calculation Engine |
||
61 | * |
||
62 | * @var Calculation |
||
63 | */ |
||
64 | private $calculationEngine; |
||
65 | |||
66 | /** |
||
67 | * Active sheet index |
||
68 | * |
||
69 | * @var integer |
||
70 | */ |
||
71 | private $activeSheetIndex = 0; |
||
72 | |||
73 | /** |
||
74 | * Named ranges |
||
75 | * |
||
76 | * @var NamedRange[] |
||
77 | */ |
||
78 | private $namedRanges = array(); |
||
79 | |||
80 | /** |
||
81 | * CellXf supervisor |
||
82 | * |
||
83 | * @var Style |
||
84 | */ |
||
85 | private $cellXfSupervisor; |
||
86 | |||
87 | /** |
||
88 | * CellXf collection |
||
89 | * |
||
90 | * @var Style[] |
||
91 | */ |
||
92 | private $cellXfCollection = array(); |
||
93 | |||
94 | /** |
||
95 | * CellStyleXf collection |
||
96 | * |
||
97 | * @var Style[] |
||
98 | */ |
||
99 | private $cellStyleXfCollection = array(); |
||
100 | |||
101 | /** |
||
102 | * hasMacros : this workbook have macros ? |
||
103 | * |
||
104 | * @var bool |
||
105 | */ |
||
106 | private $hasMacros = false; |
||
107 | |||
108 | /** |
||
109 | * macrosCode : all macros code (the vbaProject.bin file, this include form, code, etc.), null if no macro |
||
110 | * |
||
111 | * @var binary |
||
112 | */ |
||
113 | private $macrosCode; |
||
114 | /** |
||
115 | * macrosCertificate : if macros are signed, contains vbaProjectSignature.bin file, null if not signed |
||
116 | * |
||
117 | * @var binary |
||
118 | */ |
||
119 | private $macrosCertificate; |
||
120 | |||
121 | /** |
||
122 | * ribbonXMLData : null if workbook is'nt Excel 2007 or not contain a customized UI |
||
123 | * |
||
124 | * @var null|string |
||
125 | */ |
||
126 | private $ribbonXMLData; |
||
127 | |||
128 | /** |
||
129 | * ribbonBinObjects : null if workbook is'nt Excel 2007 or not contain embedded objects (picture(s)) for Ribbon Elements |
||
130 | * ignored if $ribbonXMLData is null |
||
131 | * |
||
132 | * @var null|array |
||
133 | */ |
||
134 | private $ribbonBinObjects; |
||
135 | |||
136 | /** |
||
137 | * The workbook has macros ? |
||
138 | * |
||
139 | * @return true if workbook has macros, false if not |
||
140 | */ |
||
141 | public function hasMacros() |
||
145 | |||
146 | /** |
||
147 | * Define if a workbook has macros |
||
148 | * |
||
149 | * @param boolean $hasMacros true|false |
||
150 | */ |
||
151 | public function setHasMacros($hasMacros = false) |
||
155 | |||
156 | /** |
||
157 | * Set the macros code |
||
158 | * |
||
159 | * @param string $macroCode string|null |
||
160 | */ |
||
161 | public function setMacrosCode($macroCode = null) |
||
166 | |||
167 | /** |
||
168 | * Return the macros code |
||
169 | * |
||
170 | * @return string|null |
||
171 | */ |
||
172 | public function getMacrosCode() |
||
176 | |||
177 | /** |
||
178 | * Set the macros certificate |
||
179 | * |
||
180 | * @param string|null $Certificate |
||
181 | */ |
||
182 | public function setMacrosCertificate($certificate = null) |
||
186 | |||
187 | /** |
||
188 | * Is the project signed ? |
||
189 | * |
||
190 | * @return boolean true|false |
||
191 | */ |
||
192 | public function hasMacrosCertificate() |
||
196 | |||
197 | /** |
||
198 | * Return the macros certificate |
||
199 | * |
||
200 | * @return string|null |
||
201 | */ |
||
202 | public function getMacrosCertificate() |
||
206 | |||
207 | /** |
||
208 | * Remove all macros, certificate from spreadsheet |
||
209 | * |
||
210 | */ |
||
211 | public function discardMacros() |
||
217 | |||
218 | /** |
||
219 | * set ribbon XML data |
||
220 | * |
||
221 | */ |
||
222 | public function setRibbonXMLData($target = null, $xmlData = null) |
||
230 | |||
231 | /** |
||
232 | * retrieve ribbon XML Data |
||
233 | * |
||
234 | * return string|null|array |
||
235 | */ |
||
236 | public function getRibbonXMLData($what = 'all') //we need some constants here... |
||
254 | |||
255 | /** |
||
256 | * store binaries ribbon objects (pictures) |
||
257 | * |
||
258 | */ |
||
259 | public function setRibbonBinObjects($BinObjectsNames = null, $BinObjectsData = null) |
||
267 | /** |
||
268 | * return the extension of a filename. Internal use for a array_map callback (php<5.3 don't like lambda function) |
||
269 | * |
||
270 | */ |
||
271 | private function getExtensionOnly($ThePath) |
||
275 | |||
276 | /** |
||
277 | * retrieve Binaries Ribbon Objects |
||
278 | * |
||
279 | */ |
||
280 | public function getRibbonBinObjects($What = 'all') |
||
306 | |||
307 | /** |
||
308 | * This workbook have a custom UI ? |
||
309 | * |
||
310 | * @return true|false |
||
311 | */ |
||
312 | public function hasRibbon() |
||
316 | |||
317 | /** |
||
318 | * This workbook have additionnal object for the ribbon ? |
||
319 | * |
||
320 | * @return true|false |
||
321 | */ |
||
322 | public function hasRibbonBinObjects() |
||
326 | |||
327 | /** |
||
328 | * Check if a sheet with a specified code name already exists |
||
329 | * |
||
330 | * @param string $pSheetCodeName Name of the worksheet to check |
||
331 | * @return boolean |
||
332 | */ |
||
333 | public function sheetCodeNameExists($pSheetCodeName) |
||
337 | |||
338 | /** |
||
339 | * Get sheet by code name. Warning : sheet don't have always a code name ! |
||
340 | * |
||
341 | * @param string $pName Sheet name |
||
342 | * @return Worksheet |
||
343 | */ |
||
344 | View Code Duplication | public function getSheetByCodeName($pName = '') |
|
355 | |||
356 | /** |
||
357 | * Create a new PhpSpreadsheet with one Worksheet |
||
358 | */ |
||
359 | public function __construct() |
||
386 | |||
387 | /** |
||
388 | * Code to execute when this worksheet is unset() |
||
389 | * |
||
390 | */ |
||
391 | public function __destruct() |
||
396 | |||
397 | /** |
||
398 | * Disconnect all worksheets from this PhpSpreadsheet workbook object, |
||
399 | * typically so that the PhpSpreadsheet object can be unset |
||
400 | * |
||
401 | */ |
||
402 | public function disconnectWorksheets() |
||
412 | |||
413 | /** |
||
414 | * Return the calculation engine for this worksheet |
||
415 | * |
||
416 | * @return Calculation |
||
417 | */ |
||
418 | public function getCalculationEngine() |
||
422 | |||
423 | /** |
||
424 | * Get properties |
||
425 | * |
||
426 | * @return Document\Properties |
||
427 | */ |
||
428 | public function getProperties() |
||
432 | |||
433 | /** |
||
434 | * Set properties |
||
435 | * |
||
436 | * @param Document\Properties $pValue |
||
437 | */ |
||
438 | public function setProperties(Document\Properties $pValue) |
||
442 | |||
443 | /** |
||
444 | * Get security |
||
445 | * |
||
446 | * @return Document\Security |
||
447 | */ |
||
448 | public function getSecurity() |
||
452 | |||
453 | /** |
||
454 | * Set security |
||
455 | * |
||
456 | * @param Document\Security $pValue |
||
457 | */ |
||
458 | public function setSecurity(Document\Security $pValue) |
||
462 | |||
463 | /** |
||
464 | * Get active sheet |
||
465 | * |
||
466 | * @return Worksheet |
||
467 | * |
||
468 | * @throws Exception |
||
469 | */ |
||
470 | public function getActiveSheet() |
||
474 | |||
475 | /** |
||
476 | * Create sheet and add it to this workbook |
||
477 | * |
||
478 | * @param int|null $iSheetIndex Index where sheet should go (0,1,..., or null for last) |
||
479 | * @return Worksheet |
||
480 | * @throws Exception |
||
481 | */ |
||
482 | public function createSheet($iSheetIndex = null) |
||
488 | |||
489 | /** |
||
490 | * Check if a sheet with a specified name already exists |
||
491 | * |
||
492 | * @param string $pSheetName Name of the worksheet to check |
||
493 | * @return boolean |
||
494 | */ |
||
495 | public function sheetNameExists($pSheetName) |
||
499 | |||
500 | /** |
||
501 | * Add sheet |
||
502 | * |
||
503 | * @param Worksheet $pSheet |
||
504 | * @param int|null $iSheetIndex Index where sheet should go (0,1,..., or null for last) |
||
505 | * @return Worksheet |
||
506 | * @throws Exception |
||
507 | */ |
||
508 | public function addSheet(Worksheet $pSheet, $iSheetIndex = null) |
||
542 | |||
543 | /** |
||
544 | * Remove sheet by index |
||
545 | * |
||
546 | * @param int $pIndex Active sheet index |
||
547 | * @throws Exception |
||
548 | */ |
||
549 | public function removeSheetByIndex($pIndex = 0) |
||
566 | |||
567 | /** |
||
568 | * Get sheet by index |
||
569 | * |
||
570 | * @param int $pIndex Sheet index |
||
571 | * @return Worksheet |
||
572 | * @throws Exception |
||
573 | */ |
||
574 | public function getSheet($pIndex = 0) |
||
585 | |||
586 | /** |
||
587 | * Get all sheets |
||
588 | * |
||
589 | * @return Worksheet[] |
||
590 | */ |
||
591 | public function getAllSheets() |
||
595 | |||
596 | /** |
||
597 | * Get sheet by name |
||
598 | * |
||
599 | * @param string $pName Sheet name |
||
600 | * @return Worksheet |
||
601 | */ |
||
602 | View Code Duplication | public function getSheetByName($pName = '') |
|
613 | |||
614 | /** |
||
615 | * Get index for sheet |
||
616 | * |
||
617 | * @param Worksheet $pSheet |
||
618 | * @return Sheet index |
||
619 | * @throws Exception |
||
620 | */ |
||
621 | public function getIndex(Worksheet $pSheet) |
||
631 | |||
632 | /** |
||
633 | * Set index for sheet by sheet name. |
||
634 | * |
||
635 | * @param string $sheetName Sheet name to modify index for |
||
636 | * @param int $newIndex New index for the sheet |
||
637 | * @return New sheet index |
||
638 | * @throws Exception |
||
639 | */ |
||
640 | public function setIndexByName($sheetName, $newIndex) |
||
656 | |||
657 | /** |
||
658 | * Get sheet count |
||
659 | * |
||
660 | * @return int |
||
661 | */ |
||
662 | public function getSheetCount() |
||
666 | |||
667 | /** |
||
668 | * Get active sheet index |
||
669 | * |
||
670 | * @return int Active sheet index |
||
671 | */ |
||
672 | public function getActiveSheetIndex() |
||
676 | |||
677 | /** |
||
678 | * Set active sheet index |
||
679 | * |
||
680 | * @param int $pIndex Active sheet index |
||
681 | * @throws Exception |
||
682 | * @return Worksheet |
||
683 | */ |
||
684 | public function setActiveSheetIndex($pIndex = 0) |
||
697 | |||
698 | /** |
||
699 | * Set active sheet index by name |
||
700 | * |
||
701 | * @param string $pValue Sheet title |
||
702 | * @return Worksheet |
||
703 | * @throws Exception |
||
704 | */ |
||
705 | public function setActiveSheetIndexByName($pValue = '') |
||
714 | |||
715 | /** |
||
716 | * Get sheet names |
||
717 | * |
||
718 | * @return string[] |
||
719 | */ |
||
720 | public function getSheetNames() |
||
730 | |||
731 | /** |
||
732 | * Add external sheet |
||
733 | * |
||
734 | * @param Worksheet $pSheet External sheet to add |
||
735 | * @param int|null $iSheetIndex Index where sheet should go (0,1,..., or null for last) |
||
736 | * @throws Exception |
||
737 | * @return Worksheet |
||
738 | */ |
||
739 | public function addExternalSheet(Worksheet $pSheet, $iSheetIndex = null) |
||
764 | |||
765 | /** |
||
766 | * Get named ranges |
||
767 | * |
||
768 | * @return NamedRange[] |
||
769 | */ |
||
770 | public function getNamedRanges() |
||
774 | |||
775 | /** |
||
776 | * Add named range |
||
777 | * |
||
778 | * @param NamedRange $namedRange |
||
779 | * @return PhpSpreadsheet |
||
780 | */ |
||
781 | public function addNamedRange(NamedRange $namedRange) |
||
792 | |||
793 | /** |
||
794 | * Get named range |
||
795 | * |
||
796 | * @param string $namedRange |
||
797 | * @param Worksheet|null $pSheet Scope. Use null for global scope |
||
798 | * @return NamedRange|null |
||
799 | */ |
||
800 | public function getNamedRange($namedRange, Worksheet $pSheet = null) |
||
818 | |||
819 | /** |
||
820 | * Remove named range |
||
821 | * |
||
822 | * @param string $namedRange |
||
823 | * @param Worksheet|null $pSheet Scope: use null for global scope. |
||
824 | * @return PhpSpreadsheet |
||
825 | */ |
||
826 | public function removeNamedRange($namedRange, Worksheet $pSheet = null) |
||
839 | |||
840 | /** |
||
841 | * Get worksheet iterator |
||
842 | * |
||
843 | * @return WorksheetIterator |
||
844 | */ |
||
845 | public function getWorksheetIterator() |
||
849 | |||
850 | /** |
||
851 | * Copy workbook (!= clone!) |
||
852 | * |
||
853 | * @return PhpSpreadsheet |
||
854 | */ |
||
855 | public function copy() |
||
867 | |||
868 | /** |
||
869 | * Implement PHP __clone to create a deep clone, not just a shallow copy. |
||
870 | */ |
||
871 | public function __clone() |
||
879 | |||
880 | /** |
||
881 | * Get the workbook collection of cellXfs |
||
882 | * |
||
883 | * @return Style[] |
||
884 | */ |
||
885 | public function getCellXfCollection() |
||
889 | |||
890 | /** |
||
891 | * Get cellXf by index |
||
892 | * |
||
893 | * @param int $pIndex |
||
894 | * @return Style |
||
895 | */ |
||
896 | public function getCellXfByIndex($pIndex = 0) |
||
900 | |||
901 | /** |
||
902 | * Get cellXf by hash code |
||
903 | * |
||
904 | * @param string $pValue |
||
905 | * @return Style|false |
||
906 | */ |
||
907 | public function getCellXfByHashCode($pValue = '') |
||
916 | |||
917 | /** |
||
918 | * Check if style exists in style collection |
||
919 | * |
||
920 | * @param Style $pCellStyle |
||
921 | * @return boolean |
||
922 | */ |
||
923 | public function cellXfExists($pCellStyle = null) |
||
927 | |||
928 | /** |
||
929 | * Get default style |
||
930 | * |
||
931 | * @return Style |
||
932 | * @throws Exception |
||
933 | */ |
||
934 | public function getDefaultStyle() |
||
941 | |||
942 | /** |
||
943 | * Add a cellXf to the workbook |
||
944 | * |
||
945 | * @param Style $style |
||
946 | */ |
||
947 | public function addCellXf(Style $style) |
||
952 | |||
953 | /** |
||
954 | * Remove cellXf by index. It is ensured that all cells get their xf index updated. |
||
955 | * |
||
956 | * @param integer $pIndex Index to cellXf |
||
957 | * @throws Exception |
||
958 | */ |
||
959 | public function removeCellXfByIndex($pIndex = 0) |
||
983 | |||
984 | /** |
||
985 | * Get the cellXf supervisor |
||
986 | * |
||
987 | * @return Style |
||
988 | */ |
||
989 | public function getCellXfSupervisor() |
||
993 | |||
994 | /** |
||
995 | * Get the workbook collection of cellStyleXfs |
||
996 | * |
||
997 | * @return Style[] |
||
998 | */ |
||
999 | public function getCellStyleXfCollection() |
||
1003 | |||
1004 | /** |
||
1005 | * Get cellStyleXf by index |
||
1006 | * |
||
1007 | * @param integer $pIndex Index to cellXf |
||
1008 | * @return Style |
||
1009 | */ |
||
1010 | public function getCellStyleXfByIndex($pIndex = 0) |
||
1014 | |||
1015 | /** |
||
1016 | * Get cellStyleXf by hash code |
||
1017 | * |
||
1018 | * @param string $pValue |
||
1019 | * @return Style|false |
||
1020 | */ |
||
1021 | public function getCellStyleXfByHashCode($pValue = '') |
||
1030 | |||
1031 | /** |
||
1032 | * Add a cellStyleXf to the workbook |
||
1033 | * |
||
1034 | * @param Style $pStyle |
||
1035 | */ |
||
1036 | public function addCellStyleXf(Style $pStyle) |
||
1041 | |||
1042 | /** |
||
1043 | * Remove cellStyleXf by index |
||
1044 | * |
||
1045 | * @param integer $pIndex Index to cellXf |
||
1046 | * @throws Exception |
||
1047 | */ |
||
1048 | public function removeCellStyleXfByIndex($pIndex = 0) |
||
1056 | |||
1057 | /** |
||
1058 | * Eliminate all unneeded cellXf and afterwards update the xfIndex for all cells |
||
1059 | * and columns in the workbook |
||
1060 | */ |
||
1061 | public function garbageCollect() |
||
1136 | |||
1137 | /** |
||
1138 | * Return the unique ID value assigned to this spreadsheet workbook |
||
1139 | * |
||
1140 | * @return string |
||
1141 | */ |
||
1142 | public function getID() |
||
1146 | } |
||
1147 |
Our type inference engine has found a suspicous assignment of a value to a property. This check raises an issue when a value that can be of a mixed type is assigned to a property that is type hinted more strictly.
For example, imagine you have a variable
$accountId
that can either hold an Id object or false (if there is no account id yet). Your code now assigns that value to theid
property of an instance of theAccount
class. This class holds a proper account, so the id value must no longer be false.Either this assignment is in error or a type check should be added for that assignment.