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 WP_Text_Diff_Renderer_Table 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 WP_Text_Diff_Renderer_Table, and based on these observations, apply Extract Interface, too.
1 | <?php |
||
16 | class WP_Text_Diff_Renderer_Table extends Text_Diff_Renderer { |
||
17 | |||
18 | /** |
||
19 | * @see Text_Diff_Renderer::_leading_context_lines |
||
20 | * @var int |
||
21 | * @access public |
||
22 | * @since 2.6.0 |
||
23 | */ |
||
24 | public $_leading_context_lines = 10000; |
||
25 | |||
26 | /** |
||
27 | * @see Text_Diff_Renderer::_trailing_context_lines |
||
28 | * @var int |
||
29 | * @access public |
||
30 | * @since 2.6.0 |
||
31 | */ |
||
32 | public $_trailing_context_lines = 10000; |
||
33 | |||
34 | /** |
||
35 | * Threshold for when a diff should be saved or omitted. |
||
36 | * |
||
37 | * @var float |
||
38 | * @access protected |
||
39 | * @since 2.6.0 |
||
40 | */ |
||
41 | protected $_diff_threshold = 0.6; |
||
42 | |||
43 | /** |
||
44 | * Inline display helper object name. |
||
45 | * |
||
46 | * @var string |
||
47 | * @access protected |
||
48 | * @since 2.6.0 |
||
49 | */ |
||
50 | protected $inline_diff_renderer = 'WP_Text_Diff_Renderer_inline'; |
||
51 | |||
52 | /** |
||
53 | * Should we show the split view or not |
||
54 | * |
||
55 | * @var string |
||
56 | * @access protected |
||
57 | * @since 3.6.0 |
||
58 | */ |
||
59 | protected $_show_split_view = true; |
||
60 | |||
61 | protected $compat_fields = array( '_show_split_view', 'inline_diff_renderer', '_diff_threshold' ); |
||
62 | |||
63 | /** |
||
64 | * Constructor - Call parent constructor with params array. |
||
65 | * |
||
66 | * This will set class properties based on the key value pairs in the array. |
||
67 | * |
||
68 | * @since 2.6.0 |
||
69 | * |
||
70 | * @param array $params |
||
71 | */ |
||
72 | public function __construct( $params = array() ) { |
||
77 | |||
78 | /** |
||
79 | * @ignore |
||
80 | * |
||
81 | * @param string $header |
||
82 | * @return string |
||
83 | */ |
||
84 | public function _startBlock( $header ) { |
||
87 | |||
88 | /** |
||
89 | * @ignore |
||
90 | * |
||
91 | * @param array $lines |
||
92 | * @param string $prefix |
||
93 | */ |
||
94 | public function _lines( $lines, $prefix=' ' ) { |
||
96 | |||
97 | /** |
||
98 | * @ignore |
||
99 | * |
||
100 | * @param string $line HTML-escape the value. |
||
101 | * @return string |
||
102 | */ |
||
103 | public function addedLine( $line ) { |
||
107 | |||
108 | /** |
||
109 | * @ignore |
||
110 | * |
||
111 | * @param string $line HTML-escape the value. |
||
112 | * @return string |
||
113 | */ |
||
114 | public function deletedLine( $line ) { |
||
117 | |||
118 | /** |
||
119 | * @ignore |
||
120 | * |
||
121 | * @param string $line HTML-escape the value. |
||
122 | * @return string |
||
123 | */ |
||
124 | public function contextLine( $line ) { |
||
127 | |||
128 | /** |
||
129 | * @ignore |
||
130 | * |
||
131 | * @return string |
||
132 | */ |
||
133 | public function emptyLine() { |
||
136 | |||
137 | /** |
||
138 | * @ignore |
||
139 | * @access public |
||
140 | * |
||
141 | * @param array $lines |
||
142 | * @param bool $encode |
||
143 | * @return string |
||
144 | */ |
||
145 | View Code Duplication | public function _added( $lines, $encode = true ) { |
|
175 | |||
176 | /** |
||
177 | * @ignore |
||
178 | * @access public |
||
179 | * |
||
180 | * @param array $lines |
||
181 | * @param bool $encode |
||
182 | * @return string |
||
183 | */ |
||
184 | View Code Duplication | public function _deleted( $lines, $encode = true ) { |
|
202 | |||
203 | /** |
||
204 | * @ignore |
||
205 | * @access public |
||
206 | * |
||
207 | * @param array $lines |
||
208 | * @param bool $encode |
||
209 | * @return string |
||
210 | */ |
||
211 | View Code Duplication | public function _context( $lines, $encode = true ) { |
|
228 | |||
229 | /** |
||
230 | * Process changed lines to do word-by-word diffs for extra highlighting. |
||
231 | * |
||
232 | * (TRAC style) sometimes these lines can actually be deleted or added rows. |
||
233 | * We do additional processing to figure that out |
||
234 | * |
||
235 | * @access public |
||
236 | * @since 2.6.0 |
||
237 | * |
||
238 | * @param array $orig |
||
239 | * @param array $final |
||
240 | * @return string |
||
241 | */ |
||
242 | public function _changed( $orig, $final ) { |
||
318 | |||
319 | /** |
||
320 | * Takes changed blocks and matches which rows in orig turned into which rows in final. |
||
321 | * |
||
322 | * Returns |
||
323 | * *_matches ( which rows match with which ) |
||
324 | * *_rows ( order of rows in each column interleaved with blank rows as |
||
325 | * necessary ) |
||
326 | * |
||
327 | * @since 2.6.0 |
||
328 | * |
||
329 | * @param array $orig |
||
330 | * @param array $final |
||
331 | * @return array |
||
332 | */ |
||
333 | public function interleave_changed_lines( $orig, $final ) { |
||
412 | |||
413 | /** |
||
414 | * Computes a number that is intended to reflect the "distance" between two strings. |
||
415 | * |
||
416 | * @since 2.6.0 |
||
417 | * |
||
418 | * @param string $string1 |
||
419 | * @param string $string2 |
||
420 | * @return int |
||
421 | */ |
||
422 | public function compute_string_distance( $string1, $string2 ) { |
||
437 | |||
438 | /** |
||
439 | * @ignore |
||
440 | * @since 2.6.0 |
||
441 | * |
||
442 | * @param int $a |
||
443 | * @param int $b |
||
444 | * @return int |
||
445 | */ |
||
446 | public function difference( $a, $b ) { |
||
449 | |||
450 | /** |
||
451 | * Make private properties readable for backward compatibility. |
||
452 | * |
||
453 | * @since 4.0.0 |
||
454 | * @access public |
||
455 | * |
||
456 | * @param string $name Property to get. |
||
457 | * @return mixed Property. |
||
458 | */ |
||
459 | public function __get( $name ) { |
||
464 | |||
465 | /** |
||
466 | * Make private properties settable for backward compatibility. |
||
467 | * |
||
468 | * @since 4.0.0 |
||
469 | * @access public |
||
470 | * |
||
471 | * @param string $name Property to check if set. |
||
472 | * @param mixed $value Property value. |
||
473 | * @return mixed Newly-set property. |
||
474 | */ |
||
475 | public function __set( $name, $value ) { |
||
480 | |||
481 | /** |
||
482 | * Make private properties checkable for backward compatibility. |
||
483 | * |
||
484 | * @since 4.0.0 |
||
485 | * @access public |
||
486 | * |
||
487 | * @param string $name Property to check if set. |
||
488 | * @return bool Whether the property is set. |
||
489 | */ |
||
490 | public function __isset( $name ) { |
||
495 | |||
496 | /** |
||
497 | * Make private properties un-settable for backward compatibility. |
||
498 | * |
||
499 | * @since 4.0.0 |
||
500 | * @access public |
||
501 | * |
||
502 | * @param string $name Property to unset. |
||
503 | */ |
||
504 | public function __unset( $name ) { |
||
509 | } |
||
510 |
This checks looks for assignemnts to variables using the
list(...)
function, where not all assigned variables are subsequently used.Consider the following code example.
Only the variables
$a
and$c
are used. There was no need to assign$b
.Instead, the list call could have been.