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 | 38 | 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) |
1 ignored issue
–
show
|
|||
45 | { |
||
46 | $sql = 'SELECT * |
||
47 | 4 | FROM ' . $this->ads_table . ' |
|
48 | 4 | WHERE ad_id = ' . (int) $ad_id; |
|
49 | 4 | $result = $this->db->sql_query($sql); |
|
50 | 4 | $data = $this->db->sql_fetchrow($result); |
|
51 | 4 | $this->db->sql_freeresult($result); |
|
52 | |||
53 | 4 | return $data; |
|
54 | } |
||
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) |
1 ignored issue
–
show
|
|||
157 | { |
||
158 | 1 | $ad_locations = array(); |
|
159 | |||
160 | $sql = 'SELECT location_id |
||
161 | 1 | FROM ' . $this->ad_locations_table . ' |
|
162 | 1 | WHERE ad_id = ' . (int) $ad_id; |
|
163 | 1 | $result = $this->db->sql_query($sql); |
|
164 | 1 | while ($row = $this->db->sql_fetchrow($result)) |
|
165 | { |
||
166 | 1 | $ad_locations[] = $row['location_id']; |
|
167 | 1 | } |
|
168 | 1 | $this->db->sql_freeresult($result); |
|
169 | |||
170 | 1 | return $ad_locations; |
|
171 | } |
||
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 | 2 | View Code Duplication | public function load_memberships($user_id) |
1 ignored issue
–
show
|
|||
213 | { |
||
214 | 2 | $memberships = array(); |
|
215 | $sql = 'SELECT group_id |
||
216 | 2 | FROM ' . USER_GROUP_TABLE . ' |
|
217 | 2 | WHERE user_id = ' . (int) $user_id . ' |
|
218 | 2 | AND user_pending = 0'; |
|
219 | 2 | $result = $this->db->sql_query($sql, 3600); |
|
220 | 2 | while ($row = $this->db->sql_fetchrow($result)) |
|
221 | { |
||
222 | 2 | $memberships[] = $row['group_id']; |
|
223 | 2 | } |
|
224 | 2 | $this->db->sql_freeresult($result); |
|
225 | 2 | return $memberships; |
|
226 | } |
||
227 | |||
228 | /** |
||
229 | * Load all board groups |
||
230 | * |
||
231 | * @return array List of groups |
||
232 | */ |
||
233 | 2 | View Code Duplication | public function load_groups() |
1 ignored issue
–
show
|
|||
234 | { |
||
235 | $sql = 'SELECT group_id, group_name, group_type |
||
236 | 2 | FROM ' . GROUPS_TABLE . " |
|
237 | 2 | ORDER BY group_name ASC"; |
|
238 | 2 | $result = $this->db->sql_query($sql); |
|
239 | 2 | $groups = $this->db->sql_fetchrowset($result); |
|
240 | 2 | $this->db->sql_freeresult($result); |
|
241 | |||
242 | 2 | return $groups; |
|
243 | } |
||
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 |
Duplicated code is one of the most pungent code smells. If you need to duplicate the same code in three or more different places, we strongly encourage you to look into extracting the code into a single class or operation.
You can also find more detailed suggestions in the “Code” section of your repository.