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 CSV 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 CSV, and based on these observations, apply Extract Interface, too.
1 | <?php |
||
18 | class CSV { |
||
19 | |||
20 | /** |
||
21 | * The headers in the CSV data. |
||
22 | * |
||
23 | * @var string[] |
||
24 | */ |
||
25 | public $headers; |
||
26 | |||
27 | /** |
||
28 | * Two-dimenstional integer-indexed array of the CSV's data. |
||
29 | * |
||
30 | * @var array[] |
||
31 | */ |
||
32 | public $data; |
||
33 | |||
34 | /** |
||
35 | * Temporary identifier for CSV file. |
||
36 | * |
||
37 | * @var string |
||
38 | */ |
||
39 | public $hash = false; |
||
40 | |||
41 | /** |
||
42 | * The filesystem. |
||
43 | * |
||
44 | * @var \WP_Filesystem_Base |
||
45 | */ |
||
46 | protected $filesystem; |
||
47 | |||
48 | /** |
||
49 | * Create a new CSV object based on a file. |
||
50 | * |
||
51 | * 1. If a file is being uploaded (i.e. `$_FILES['file']` is set), attempt |
||
52 | * to use it as the CSV file. |
||
53 | * 2. On the otherhand, if we're given a hash, attempt to use this to locate |
||
54 | * a local temporary file. |
||
55 | * |
||
56 | * In either case, if a valid CSV file cannot be found and parsed, throw an |
||
57 | * exception. |
||
58 | * |
||
59 | * @param \WP_Filesystem_Base $filesystem The filesystem object. |
||
60 | * @param string|boolean $hash The hash of an in-progress import, or false. |
||
61 | * @param string[]|boolean $uploaded The result of wp_handle_upload(), or false. |
||
62 | */ |
||
63 | public function __construct( $filesystem, $hash = false, $uploaded = false ) { |
||
75 | |||
76 | /** |
||
77 | * Check the (already-handled) upload and rename the uploaded file. |
||
78 | * |
||
79 | * @see wp_handle_upload() |
||
80 | * @param array $uploaded The array detailing the uploaded file. |
||
81 | * @throws \Exception On upload error or if the file isn't a CSV. |
||
82 | */ |
||
83 | private function save_file( $uploaded ) { |
||
94 | |||
95 | /** |
||
96 | * Load CSV data from the file identified by the current hash. If no hash is |
||
97 | * set, this method does nothing. |
||
98 | * |
||
99 | * @return void |
||
100 | * @throws \Exception If the hash-identified file doesn't exist. |
||
101 | */ |
||
102 | public function load_data() { |
||
122 | |||
123 | /** |
||
124 | * Get the number of data rows in the file (i.e. excluding the header row). |
||
125 | * |
||
126 | * @return integer The number of rows. |
||
127 | */ |
||
128 | public function row_count() { |
||
131 | |||
132 | /** |
||
133 | * Whether or not a file has been successfully loaded. |
||
134 | * |
||
135 | * @return boolean |
||
136 | */ |
||
137 | public function loaded() { |
||
140 | |||
141 | /** |
||
142 | * Take a mapping of DB column name to CSV column name, and convert it to |
||
143 | * a mapping of CSV column number to DB column name. This ignores empty |
||
144 | * column headers in the CSV (so we don't have to distinguish between |
||
145 | * not-matching and matching-on-empty-string). |
||
146 | * |
||
147 | * @param string[] $column_map The map from column headings to indices. |
||
148 | * @return array Keys are CSV indexes, values are DB column names |
||
149 | */ |
||
150 | private function remap( $column_map ) { |
||
162 | |||
163 | /** |
||
164 | * Rename all keys in all data rows to match DB column names, and normalize |
||
165 | * all values to be valid for the `$table`. |
||
166 | * |
||
167 | * If a _value_ in the array matches a lowercased DB column header, the _key_ |
||
168 | * of that value is the DB column name to which that header has been matched. |
||
169 | * |
||
170 | * @param DB\Table $table The table object. |
||
171 | * @param array $column_map Associating the headings to the indices. |
||
172 | * @return array Array of error messages. |
||
173 | */ |
||
174 | public function match_fields( $table, $column_map ) { |
||
240 | |||
241 | /** |
||
242 | * Assume all data is now valid, and only FK values remain to be translated. |
||
243 | * |
||
244 | * @param DB\Table $table The table into which to import data. |
||
245 | * @param array $column_map array of DB names to import names. |
||
246 | * @return integer The number of rows imported. |
||
247 | */ |
||
248 | public function import_data( $table, $column_map ) { |
||
283 | |||
284 | /** |
||
285 | * Determine whether a given value is valid for a foreign key (i.e. is the |
||
286 | * title of a foreign row). |
||
287 | * |
||
288 | * @param DB\Column $column The column to check in. |
||
289 | * @param string $value The value to validate. |
||
290 | * @return false|string False if the value is valid, error message otherwise. |
||
291 | */ |
||
292 | protected function validate_foreign_key( $column, $value ) { |
||
302 | |||
303 | /** |
||
304 | * Get the rows of a foreign table where the title column equals a given |
||
305 | * value. |
||
306 | * |
||
307 | * @param DB\Table $foreign_table The table from which to fetch rows. |
||
308 | * @param string $value The value to match against the title column. |
||
309 | * @return Record[] The foreign records. |
||
310 | */ |
||
311 | protected function get_fk_rows( $foreign_table, $value ) { |
||
316 | |||
317 | /** |
||
318 | * Determine whether the given value exists. |
||
319 | * |
||
320 | * @param DB\Table $table The table to check in. |
||
321 | * @param DB\Column $column The column to check. |
||
322 | * @param mixed $value The value to look for. |
||
323 | * @return boolean |
||
324 | */ |
||
325 | protected function value_exists( $table, $column, $value ) { |
||
333 | } |
||
334 |
This check looks at variables that have been passed in as parameters and are passed out again to other methods.
If the outgoing method call has stricter type requirements than the method itself, an issue is raised.
An additional type check may prevent trouble.