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 AchievementCheckListener 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 AchievementCheckListener, and based on these observations, apply Extract Interface, too.
1 | <?php |
||
13 | class AchievementCheckListener |
||
14 | { |
||
15 | protected $manager; |
||
16 | protected $tokenStorage; |
||
17 | protected $authorizationChecker; |
||
18 | |||
19 | // L'utilisateur qui tente d'obtenir l'achievement |
||
20 | protected $user; |
||
21 | |||
22 | // Liste des achievements unlockés actuellement (identifiants seulement) |
||
23 | protected $achievements = []; |
||
24 | |||
25 | public function __construct(EntityManager $manager, TokenStorage $tokenStorage, AuthorizationChecker $authorizationChecker) |
||
38 | |||
39 | private function loadUser(User $user) |
||
49 | |||
50 | // Check si un achievement donné est accompli, si oui envoie une notification |
||
51 | public function check(AchievementCheckEvent $event) |
||
104 | |||
105 | // Calcule le nombre de points de l'utilisateur |
||
106 | public function points() |
||
128 | |||
129 | // Fonctions de check correspondant aux divers achievements |
||
130 | // Attention, ne pas changer les IDs des achievements à la légère !!! |
||
131 | // Les checks qui retournent true sont en fait assez simples : |
||
132 | // L'achievement associé est donc du type "faire ça action au moins une fois" |
||
133 | |||
134 | // Ponts inside |
||
135 | // Se logger sur le site |
||
136 | public function check0() |
||
140 | |||
141 | // Fouilleur |
||
142 | // Faire le tour du site |
||
143 | public function check5() |
||
147 | |||
148 | // Photogénique |
||
149 | // Changer la photo de son profil |
||
150 | public function check10() |
||
154 | |||
155 | // Travailleur |
||
156 | // Choisir ses cours |
||
157 | public function check20() |
||
161 | |||
162 | // Autobiographie |
||
163 | // Remplir ses infos (chambre, téléphone, département, origine, nationalité...) |
||
164 | public function check30() |
||
168 | |||
169 | // Remplir ses infos étendues (stages, projets...) |
||
170 | //public function check4() { return true; } |
||
171 | |||
172 | // Smart |
||
173 | // Synchroniser le calendrier avec son téléphone |
||
174 | public function check40() |
||
178 | |||
179 | // Connecté |
||
180 | // Installer l'application mobile |
||
181 | //public function check6() { return true; } |
||
182 | |||
183 | // Downloader |
||
184 | // Télécharger un fichier sur Ponthub |
||
185 | public function check50() |
||
189 | |||
190 | // Super Downloader |
||
191 | // Télécharger plus de 100Go sur Ponthub |
||
192 | public function check60() |
||
196 | |||
197 | // Ultimate Downloader |
||
198 | // Télécharger plus de 500Go sur Ponthub |
||
199 | public function check70() |
||
203 | |||
204 | private function totalPontHubSize() |
||
214 | |||
215 | // Will be there ! |
||
216 | // Participer à un event |
||
217 | public function check80() |
||
221 | |||
222 | // Pookie |
||
223 | // Uploader un fichier d'annale |
||
224 | public function check90() |
||
228 | |||
229 | // Spirit |
||
230 | // Être membre d'un club |
||
231 | public function check100() |
||
237 | |||
238 | // Nouvelliste |
||
239 | // Écrire une news pour un club |
||
240 | public function check110() |
||
244 | |||
245 | // Organisateur |
||
246 | // Créer un event pour un club |
||
247 | public function check120() |
||
251 | |||
252 | // Ruiné |
||
253 | // Avoir un solde foyer négatif |
||
254 | View Code Duplication | public function check130() |
|
269 | |||
270 | // Ruiné |
||
271 | // Avoir un solde foyer positif |
||
272 | View Code Duplication | public function check140() |
|
287 | |||
288 | // Non, ce n'était pas "password1234" |
||
289 | // Oublier son mot de passe |
||
290 | public function check150() |
||
294 | |||
295 | // The Game |
||
296 | // Jouer à La Réponse D |
||
297 | public function check153() |
||
301 | |||
302 | // Puceau, pas puceau |
||
303 | // Réussir 100% sur la promo d'en dessous dans La Réponse D |
||
304 | public function check154() |
||
308 | |||
309 | // Connaisseur |
||
310 | // Réussir un 100% sur sa promo dans La Réponse D |
||
311 | public function check155() |
||
315 | |||
316 | // Bientôt vieux cons |
||
317 | // Réussir un 100% sur la promo d'au dessus dans La Réponse D |
||
318 | public function check156() |
||
322 | |||
323 | // JRP'1747 |
||
324 | // Réussir un 100% en mode hardcore sur une promo de vieux dans La Réponse D |
||
325 | public function check157() |
||
329 | |||
330 | // H3LLLP UPON SA BEUG!!!! |
||
331 | // Reporter un bug |
||
332 | public function check160() |
||
336 | |||
337 | // Technophobe |
||
338 | // Contacter le KI pour un dépannage matériel/logiciel |
||
339 | public function check170() |
||
343 | |||
344 | // KIen |
||
345 | // Faire partie du KI |
||
346 | public function check180() |
||
354 | |||
355 | // Appelez-moi Dieu |
||
356 | // Être admin |
||
357 | public function check190() |
||
361 | |||
362 | // Unlocker |
||
363 | // Compléter 10 achievements |
||
364 | public function check200() |
||
368 | |||
369 | // Crazy Unlocker |
||
370 | // Compléter 50% des achievements |
||
371 | public function check210() |
||
375 | |||
376 | // Total Unlocker |
||
377 | // Compléter 90% des achievements |
||
378 | public function check220() |
||
382 | } |
||
383 |
The
EntityManager
might become unusable for example if a transaction is rolled back and it gets closed. Let’s assume that somewhere in your application, or in a third-party library, there is code such as the following:If that code throws an exception and the
EntityManager
is closed. Any other code which depends on the same instance of theEntityManager
during this request will fail.On the other hand, if you instead inject the
ManagerRegistry
, thegetManager()
method guarantees that you will always get a usable manager instance.