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:
1 | <?php |
||
13 | class manager |
||
14 | { |
||
15 | /** @var \phpbb\db\driver\driver_interface */ |
||
16 | protected $db; |
||
17 | |||
18 | /** @var string */ |
||
19 | protected $ads_table; |
||
20 | |||
21 | /** @var string */ |
||
22 | protected $ad_locations_table; |
||
23 | |||
24 | /** |
||
25 | * Constructor |
||
26 | * |
||
27 | * @param \phpbb\db\driver\driver_interface $db DB driver interface |
||
28 | * @param string $ads_table Ads table |
||
29 | * @param string $ad_locations_table Ad locations table |
||
30 | */ |
||
31 | 40 | public function __construct(\phpbb\db\driver\driver_interface $db, $ads_table, $ad_locations_table) |
|
37 | |||
38 | /** |
||
39 | * Get specific ad |
||
40 | * |
||
41 | * @param int $ad_id Advertisement ID |
||
42 | * @return mixed Array with advertisement data, false if ad doesn't exist |
||
43 | */ |
||
44 | 4 | View Code Duplication | public function get_ad($ad_id) |
55 | |||
56 | /** |
||
57 | * Get one ad per every location |
||
58 | * |
||
59 | * @param array $ad_locations List of ad locations to fetch ads for |
||
60 | * @return array List of ad codes for each location |
||
61 | */ |
||
62 | 1 | public function get_ads($ad_locations) |
|
83 | |||
84 | /** |
||
85 | * Get all advertisements |
||
86 | * |
||
87 | * @return array List of all ads |
||
88 | */ |
||
89 | 1 | View Code Duplication | public function get_all_ads() |
99 | |||
100 | /** |
||
101 | * Insert new advertisement to the database |
||
102 | * |
||
103 | * @param array $data New ad data |
||
104 | * @return int New advertisement ID |
||
105 | */ |
||
106 | 1 | View Code Duplication | public function insert_ad($data) |
115 | |||
116 | /** |
||
117 | * Update advertisement |
||
118 | * |
||
119 | * @param int $ad_id Advertisement ID |
||
120 | * @param array $data List of data to update in the database |
||
121 | * @return int Number of affected rows. Can be used to determine if any ad has been updated. |
||
122 | */ |
||
123 | 7 | View Code Duplication | public function update_ad($ad_id, $data) |
134 | |||
135 | /** |
||
136 | * Delete advertisement |
||
137 | * |
||
138 | * @param int $ad_id Advertisement ID |
||
139 | * @return int Number of affected rows. Can be used to determine if any ad has been deleted. |
||
140 | */ |
||
141 | 2 | public function delete_ad($ad_id) |
|
149 | |||
150 | /** |
||
151 | * Get all locations for specified advertisement |
||
152 | * |
||
153 | * @param int $ad_id Advertisement ID |
||
154 | * @return array List of template locations for specified ad |
||
155 | */ |
||
156 | 1 | View Code Duplication | public function get_ad_locations($ad_id) |
172 | |||
173 | /** |
||
174 | * Insert advertisement locations |
||
175 | * |
||
176 | * @param int $ad_id Advertisement ID |
||
177 | * @param array $ad_locations List of template locations for this ad |
||
178 | * @return void |
||
179 | */ |
||
180 | 2 | public function insert_ad_locations($ad_id, $ad_locations) |
|
192 | |||
193 | /** |
||
194 | * Delete advertisement locations |
||
195 | * |
||
196 | * @param int $ad_id Advertisement ID |
||
197 | * @return void |
||
198 | */ |
||
199 | 3 | public function delete_ad_locations($ad_id) |
|
205 | |||
206 | /** |
||
207 | * Load memberships of the user |
||
208 | * |
||
209 | * @param int $user_id User ID to load memberships |
||
210 | * @return array List of group IDs user is member of |
||
211 | */ |
||
212 | 4 | View Code Duplication | public function load_memberships($user_id) |
227 | |||
228 | /** |
||
229 | * Load all board groups |
||
230 | * |
||
231 | * @return array List of groups |
||
232 | */ |
||
233 | 2 | View Code Duplication | public function load_groups() |
244 | |||
245 | /** |
||
246 | * Make sure only necessary data make their way to SQL query |
||
247 | * |
||
248 | * @param array $data List of data to query the database |
||
249 | * @return array Cleaned data that contain only valid keys |
||
250 | */ |
||
251 | 8 | protected function intersect_ad_data($data) |
|
261 | |||
262 | /** |
||
263 | * Get the random statement for this database layer |
||
264 | * |
||
265 | * @return string Random statement for current database layer |
||
266 | */ |
||
267 | 1 | protected function sql_random() |
|
287 | } |
||
288 |
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.