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 Page 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 Page, and based on these observations, apply Extract Interface, too.
1 | <?php |
||
24 | class Page extends AbstractFrameDecorator |
||
25 | { |
||
26 | |||
27 | /** |
||
28 | * y value of bottom page margin |
||
29 | * |
||
30 | * @var float |
||
31 | */ |
||
32 | protected $_bottom_page_margin; |
||
33 | |||
34 | /** |
||
35 | * Flag indicating page is full. |
||
36 | * |
||
37 | * @var bool |
||
38 | */ |
||
39 | protected $_page_full; |
||
40 | |||
41 | /** |
||
42 | * Number of tables currently being reflowed |
||
43 | * |
||
44 | * @var int |
||
45 | */ |
||
46 | protected $_in_table; |
||
47 | |||
48 | /** |
||
49 | * The pdf renderer |
||
50 | * |
||
51 | * @var Renderer |
||
52 | */ |
||
53 | protected $_renderer; |
||
54 | |||
55 | /** |
||
56 | * This page's floating frames |
||
57 | * |
||
58 | * @var array |
||
59 | */ |
||
60 | protected $_floating_frames = array(); |
||
61 | |||
62 | //........................................................................ |
||
|
|||
63 | |||
64 | /** |
||
65 | * Class constructor |
||
66 | * |
||
67 | * @param Frame $frame the frame to decorate |
||
68 | * @param Dompdf $dompdf |
||
69 | */ |
||
70 | function __construct(Frame $frame, Dompdf $dompdf) |
||
77 | |||
78 | /** |
||
79 | * Set the renderer used for this pdf |
||
80 | * |
||
81 | * @param Renderer $renderer the renderer to use |
||
82 | */ |
||
83 | function set_renderer($renderer) |
||
87 | |||
88 | /** |
||
89 | * Return the renderer used for this pdf |
||
90 | * |
||
91 | * @return Renderer |
||
92 | */ |
||
93 | function get_renderer() |
||
97 | |||
98 | /** |
||
99 | * Set the frame's containing block. Overridden to set $this->_bottom_page_margin. |
||
100 | * |
||
101 | * @param float $x |
||
102 | * @param float $y |
||
103 | * @param float $w |
||
104 | * @param float $h |
||
105 | */ |
||
106 | function set_containing_block($x = null, $y = null, $w = null, $h = null) |
||
114 | |||
115 | /** |
||
116 | * Returns true if the page is full and is no longer accepting frames. |
||
117 | * |
||
118 | * @return bool |
||
119 | */ |
||
120 | function is_full() |
||
124 | |||
125 | /** |
||
126 | * Start a new page by resetting the full flag. |
||
127 | */ |
||
128 | function next_page() |
||
134 | |||
135 | /** |
||
136 | * Indicate to the page that a table is currently being reflowed. |
||
137 | */ |
||
138 | function table_reflow_start() |
||
142 | |||
143 | /** |
||
144 | * Indicate to the page that table reflow is finished. |
||
145 | */ |
||
146 | function table_reflow_end() |
||
150 | |||
151 | /** |
||
152 | * Return whether we are currently in a nested table or not |
||
153 | * |
||
154 | * @return bool |
||
155 | */ |
||
156 | function in_nested_table() |
||
160 | |||
161 | /** |
||
162 | * Check if a forced page break is required before $frame. This uses the |
||
163 | * frame's page_break_before property as well as the preceeding frame's |
||
164 | * page_break_after property. |
||
165 | * |
||
166 | * @link http://www.w3.org/TR/CSS21/page.html#forced |
||
167 | * |
||
168 | * @param Frame $frame the frame to check |
||
169 | * |
||
170 | * @return bool true if a page break occured |
||
171 | */ |
||
172 | function check_forced_page_break(Frame $frame) |
||
229 | |||
230 | /** |
||
231 | * Determine if a page break is allowed before $frame |
||
232 | * http://www.w3.org/TR/CSS21/page.html#allowed-page-breaks |
||
233 | * |
||
234 | * In the normal flow, page breaks can occur at the following places: |
||
235 | * |
||
236 | * 1. In the vertical margin between block boxes. When a page |
||
237 | * break occurs here, the used values of the relevant |
||
238 | * 'margin-top' and 'margin-bottom' properties are set to '0'. |
||
239 | * 2. Between line boxes inside a block box. |
||
240 | * |
||
241 | * These breaks are subject to the following rules: |
||
242 | * |
||
243 | * * Rule A: Breaking at (1) is allowed only if the |
||
244 | * 'page-break-after' and 'page-break-before' properties of |
||
245 | * all the elements generating boxes that meet at this margin |
||
246 | * allow it, which is when at least one of them has the value |
||
247 | * 'always', 'left', or 'right', or when all of them are |
||
248 | * 'auto'. |
||
249 | * |
||
250 | * * Rule B: However, if all of them are 'auto' and the |
||
251 | * nearest common ancestor of all the elements has a |
||
252 | * 'page-break-inside' value of 'avoid', then breaking here is |
||
253 | * not allowed. |
||
254 | * |
||
255 | * * Rule C: Breaking at (2) is allowed only if the number of |
||
256 | * line boxes between the break and the start of the enclosing |
||
257 | * block box is the value of 'orphans' or more, and the number |
||
258 | * of line boxes between the break and the end of the box is |
||
259 | * the value of 'widows' or more. |
||
260 | * |
||
261 | * * Rule D: In addition, breaking at (2) is allowed only if |
||
262 | * the 'page-break-inside' property is 'auto'. |
||
263 | * |
||
264 | * If the above doesn't provide enough break points to keep |
||
265 | * content from overflowing the page boxes, then rules B and D are |
||
266 | * dropped in order to find additional breakpoints. |
||
267 | * |
||
268 | * If that still does not lead to sufficient break points, rules A |
||
269 | * and C are dropped as well, to find still more break points. |
||
270 | * |
||
271 | * We will also allow breaks between table rows. However, when |
||
272 | * splitting a table, the table headers should carry over to the |
||
273 | * next page (but they don't yet). |
||
274 | * |
||
275 | * @param Frame $frame the frame to check |
||
276 | * |
||
277 | * @return bool true if a break is allowed, false otherwise |
||
278 | */ |
||
279 | protected function _page_break_allowed(Frame $frame) |
||
452 | |||
453 | /** |
||
454 | * Check if $frame will fit on the page. If the frame does not fit, |
||
455 | * the frame tree is modified so that a page break occurs in the |
||
456 | * correct location. |
||
457 | * |
||
458 | * @param Frame $frame the frame to check |
||
459 | * |
||
460 | * @return bool |
||
461 | */ |
||
462 | function check_page_break(Frame $frame) |
||
625 | |||
626 | //........................................................................ |
||
627 | |||
628 | /** |
||
629 | * @param Frame|null $frame |
||
630 | * @param bool $force_pagebreak |
||
631 | */ |
||
632 | function split(Frame $frame = null, $force_pagebreak = false) |
||
636 | |||
637 | /** |
||
638 | * Add a floating frame |
||
639 | * |
||
640 | * @param Frame $frame |
||
641 | * |
||
642 | * @return void |
||
643 | */ |
||
644 | function add_floating_frame(Frame $frame) |
||
648 | |||
649 | /** |
||
650 | * @return Frame[] |
||
651 | */ |
||
652 | function get_floating_frames() |
||
656 | |||
657 | /** |
||
658 | * @param $key |
||
659 | */ |
||
660 | public function remove_floating_frame($key) |
||
664 | |||
665 | /** |
||
666 | * @param Frame $child |
||
667 | * @return int|mixed |
||
668 | */ |
||
669 | public function get_lowest_float_offset(Frame $child) |
||
692 | } |
||
693 |
Sometimes obsolete code just ends up commented out instead of removed. In this case it is better to remove the code once you have checked you do not need it.
The code might also have been commented out for debugging purposes. In this case it is vital that someone uncomments it again or your project may behave in very unexpected ways in production.
This check looks for comments that seem to be mostly valid code and reports them.