Complex classes like Media 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 Media, and based on these observations, apply Extract Interface, too.
1 | <?php |
||
26 | class Media |
||
27 | { |
||
28 | const MEDIA_TYPE_AUDIO = 1; |
||
29 | const MEDIA_TYPE_VIDEO = 2; |
||
30 | /** |
||
31 | * @var string |
||
32 | * |
||
33 | * @ORM\Column(type="string", length=255, nullable=true) |
||
34 | * @Groups({"media-read"}) |
||
35 | */ |
||
36 | protected $artist; |
||
37 | |||
38 | /** |
||
39 | * @var int |
||
40 | * |
||
41 | * @ORM\Column(type="integer", nullable=true) |
||
42 | * @Groups({"media-read"}) |
||
43 | */ |
||
44 | protected $bpm; |
||
45 | /** |
||
46 | * @var string |
||
47 | * |
||
48 | * @ORM\Column(type="text", nullable=true) |
||
49 | * @Groups({"media-read"}) |
||
50 | * |
||
51 | */ |
||
52 | protected $fullPath; |
||
53 | /** |
||
54 | * @var string |
||
55 | * |
||
56 | * @ORM\Column(type="string", length=32, nullable=false) |
||
57 | * @Groups({"media-read"}) |
||
58 | * |
||
59 | */ |
||
60 | protected $fullFilePathMd5; |
||
61 | /** |
||
62 | * @var string |
||
63 | * |
||
64 | * @ORM\Column(type="string", length=200, nullable=true) |
||
65 | * @Groups({"media-read"}) |
||
66 | * |
||
67 | */ |
||
68 | protected $dirName; |
||
69 | /** |
||
70 | * @var string |
||
71 | * |
||
72 | * @ORM\Column(type="string", length=255, nullable=true) |
||
73 | * @Groups({"media-read", "artist-read", "genre-read"}) |
||
74 | */ |
||
75 | protected $title; |
||
76 | /** |
||
77 | * @var \DateTime |
||
78 | * |
||
79 | * @ORM\Column(type="datetime", nullable=true) |
||
80 | * @Groups({"media-read"}) |
||
81 | */ |
||
82 | protected $releaseDate; |
||
83 | /** |
||
84 | * @var string |
||
85 | * |
||
86 | * @ORM\Column(type="string", length=40, nullable=true) |
||
87 | * @Groups({"media-read"}) |
||
88 | */ |
||
89 | protected $version; |
||
90 | /** |
||
91 | * @var string |
||
92 | * |
||
93 | * @ORM\Column(type="string", length=255, nullable=true) |
||
94 | * @Groups({"media-read"}) |
||
95 | */ |
||
96 | protected $fileName; |
||
97 | /** |
||
98 | * @var string |
||
99 | * |
||
100 | * @ORM\Column(type="boolean", nullable=false, options={"default":false}) |
||
101 | * @Groups({"media-read"}) |
||
102 | */ |
||
103 | protected $exist = false; |
||
104 | /** |
||
105 | * @var bool |
||
106 | * @todo remove property and associated method |
||
107 | * @ORM\Column(type="boolean", nullable=false, options={"default":false}) |
||
108 | * @Groups({"media-read", "genre-read", "artist-read"}) |
||
109 | */ |
||
110 | protected $tagged = false; |
||
111 | /** |
||
112 | * @var string |
||
113 | * |
||
114 | * @ORM\Column(type="integer", nullable=true) |
||
115 | * @Groups({"media-read"}) |
||
116 | */ |
||
117 | protected $score = 0; |
||
118 | /** |
||
119 | * @var \DateTime |
||
120 | * |
||
121 | * @ORM\Column(type="datetime", nullable=true) |
||
122 | * @Groups({"media-read"}) |
||
123 | */ |
||
124 | protected $deletedAt; |
||
125 | /** |
||
126 | * @var integer |
||
127 | * |
||
128 | * @ORM\Column(name="id", type="integer") |
||
129 | * @ORM\Id |
||
130 | * @ORM\GeneratedValue(strategy="AUTO") |
||
131 | */ |
||
132 | protected $id; |
||
133 | /** |
||
134 | * @ORM\ManyToMany(targetEntity="\AudioCoreEntity\Entity\Genre", inversedBy="medias", cascade={"persist", "detach", "refresh"}, fetch="EXTRA_LAZY") |
||
135 | * @ORM\JoinTable(name="media_genre", |
||
136 | * joinColumns={@ORM\JoinColumn(name="media_id", referencedColumnName="id")}, |
||
137 | * inverseJoinColumns={@ORM\JoinColumn(name="genre_id", referencedColumnName="id")} |
||
138 | * ) |
||
139 | * @var ArrayCollection<Genre> |
||
140 | * @Groups({"media-read"}) |
||
141 | **/ |
||
142 | protected $genres; |
||
143 | /** |
||
144 | * @ORM\ManyToMany(targetEntity="\AudioCoreEntity\Entity\Artist", inversedBy="medias", cascade={"persist", "detach", "refresh"}, fetch="EXTRA_LAZY") |
||
145 | * @ORM\JoinTable( |
||
146 | * joinColumns={@ORM\JoinColumn(name="media_id", referencedColumnName="id")}, |
||
147 | * inverseJoinColumns={@ORM\JoinColumn(name="artist_id", referencedColumnName="id")} |
||
148 | * ) |
||
149 | * @var ArrayCollection<Artist> |
||
150 | * @Groups({"media-read"}) |
||
151 | **/ |
||
152 | protected $artists; |
||
153 | /** |
||
154 | * @var integer |
||
155 | * |
||
156 | * @ORM\Column(type="integer", nullable=true) |
||
157 | * @Groups({"media-read"}) |
||
158 | */ |
||
159 | protected $type; |
||
160 | /** |
||
161 | * @var integer |
||
162 | * |
||
163 | * @ORM\Column(type="integer", length=4, nullable=true) |
||
164 | * @Groups({"media-read", "artist-read", "genre-read"}) |
||
165 | */ |
||
166 | protected $year; |
||
167 | /** |
||
168 | * |
||
169 | */ |
||
170 | 8 | public function __construct() |
|
175 | |||
176 | 3 | public static function getTypes() |
|
183 | |||
184 | /** |
||
185 | * Get id |
||
186 | * |
||
187 | * @return integer |
||
188 | */ |
||
189 | 1 | public function getId() |
|
193 | |||
194 | /** |
||
195 | * Get artist. |
||
196 | * |
||
197 | * @return string |
||
198 | */ |
||
199 | 1 | public function getArtist() |
|
203 | |||
204 | /** |
||
205 | * Set artist. |
||
206 | * |
||
207 | * @param string $artist |
||
208 | * |
||
209 | * @return Media |
||
210 | */ |
||
211 | 1 | public function setArtist($artist) |
|
219 | |||
220 | /** |
||
221 | * @return int |
||
222 | */ |
||
223 | 2 | public function getBpm() |
|
227 | |||
228 | /** |
||
229 | * @param int $bpm |
||
230 | * @return Media |
||
231 | */ |
||
232 | 2 | public function setBpm($bpm) |
|
241 | |||
242 | /** |
||
243 | * @return string |
||
244 | */ |
||
245 | 1 | public function getExist() |
|
249 | |||
250 | /** |
||
251 | * @param string $exist |
||
252 | * @return $this |
||
253 | */ |
||
254 | 1 | public function setExist($exist) |
|
259 | |||
260 | /** |
||
261 | * Get fullPath. |
||
262 | * |
||
263 | * @return string |
||
264 | */ |
||
265 | 1 | public function getFullPath() |
|
269 | |||
270 | /** |
||
271 | * Set fullPath. |
||
272 | * |
||
273 | * @param string $fullPath |
||
274 | * |
||
275 | * @return Media |
||
276 | */ |
||
277 | 1 | public function setFullPath($fullPath) |
|
294 | |||
295 | /** |
||
296 | * @return string |
||
297 | */ |
||
298 | 1 | public function getFullFilePathMd5() |
|
302 | |||
303 | /** |
||
304 | * @param string $fullFilePathMd5 |
||
305 | * @return Media |
||
306 | */ |
||
307 | 1 | public function setFullFilePathMd5($fullFilePathMd5) |
|
314 | |||
315 | |||
316 | |||
317 | /** |
||
318 | * Get title. |
||
319 | * |
||
320 | * @return string |
||
321 | */ |
||
322 | 1 | public function getTitle() |
|
326 | |||
327 | /** |
||
328 | * Set title. |
||
329 | * |
||
330 | * @param string $title |
||
331 | * |
||
332 | * @return Media |
||
333 | */ |
||
334 | 1 | public function setTitle($title) |
|
342 | |||
343 | /** |
||
344 | * Get type |
||
345 | * |
||
346 | * @return integer |
||
347 | */ |
||
348 | 1 | public function getType() |
|
352 | |||
353 | /** |
||
354 | * Set type |
||
355 | * |
||
356 | * @param integer $type |
||
357 | * @return Media |
||
358 | */ |
||
359 | 2 | public function setType($type) |
|
369 | |||
370 | /** |
||
371 | * Get releaseDate. |
||
372 | * |
||
373 | * @return \DateTime |
||
374 | */ |
||
375 | 1 | public function getReleaseDate() |
|
379 | |||
380 | /** |
||
381 | * Set releaseDate. |
||
382 | * |
||
383 | * @param \DateTime $releaseDate |
||
384 | * |
||
385 | * @return Media |
||
386 | */ |
||
387 | 1 | public function setReleaseDate($releaseDate) |
|
393 | |||
394 | /** |
||
395 | * @param Genre $genre |
||
396 | * @return $this |
||
397 | */ |
||
398 | 1 | public function addGenre(Genre $genre) |
|
405 | |||
406 | /** |
||
407 | * @param Genre $genre |
||
408 | * @return $this |
||
409 | */ |
||
410 | 1 | public function removeGenre(Genre $genre) |
|
417 | |||
418 | /** |
||
419 | * @return ArrayCollection |
||
420 | */ |
||
421 | 1 | public function getGenres() |
|
425 | |||
426 | /** |
||
427 | * Set Genres. |
||
428 | * @param ArrayCollection $genres |
||
429 | * @return $this |
||
430 | */ |
||
431 | 1 | public function setGenres(ArrayCollection $genres) |
|
437 | /** |
||
438 | * @param Artist $artist |
||
439 | * @return $this |
||
440 | */ |
||
441 | 1 | public function addArtist(Artist $artist) |
|
448 | |||
449 | /** |
||
450 | * @param Genre $artist |
||
451 | * @return $this |
||
452 | */ |
||
453 | 1 | public function removeArtist(Artist $artist) |
|
460 | |||
461 | /** |
||
462 | * @return ArrayCollection |
||
463 | */ |
||
464 | 1 | public function getArtists() |
|
468 | |||
469 | /** |
||
470 | * @param ArrayCollection $artists |
||
471 | * @return $this |
||
472 | */ |
||
473 | 1 | public function setArtists(ArrayCollection $artists) |
|
479 | |||
480 | /** |
||
481 | * Get version. |
||
482 | * |
||
483 | * @return string |
||
484 | */ |
||
485 | 1 | public function getVersion() |
|
489 | |||
490 | /** |
||
491 | * Set version. |
||
492 | * |
||
493 | * @param string $version |
||
494 | * |
||
495 | * @return Media |
||
496 | */ |
||
497 | 1 | public function setVersion($version) |
|
503 | |||
504 | /** |
||
505 | * @return string |
||
506 | */ |
||
507 | 1 | public function getFileName() |
|
511 | |||
512 | /** |
||
513 | * @param string $fileName |
||
514 | * @return $this |
||
515 | */ |
||
516 | 1 | public function setFileName($fileName) |
|
522 | |||
523 | /** |
||
524 | * @return string |
||
525 | */ |
||
526 | 1 | public function getScore() |
|
530 | |||
531 | /** |
||
532 | * @param string $score |
||
533 | * @return Media |
||
534 | */ |
||
535 | 1 | public function setScore($score) |
|
542 | |||
543 | /** |
||
544 | * @return \DateTime |
||
545 | */ |
||
546 | 1 | public function getDeletedAt() |
|
550 | |||
551 | /** |
||
552 | * @param \DateTime $deletedAt |
||
553 | * @return Media |
||
554 | */ |
||
555 | 1 | public function setDeletedAt($deletedAt) |
|
560 | |||
561 | /** |
||
562 | * @return string |
||
563 | */ |
||
564 | 1 | public function getDirName() |
|
568 | |||
569 | /** |
||
570 | * @param string $dirName |
||
571 | * @return Media |
||
572 | */ |
||
573 | 1 | public function setDirName($dirName) |
|
583 | |||
584 | /** |
||
585 | * @return int |
||
586 | */ |
||
587 | 1 | public function getYear() |
|
591 | |||
592 | /** |
||
593 | * @param int $year |
||
594 | * @return Media |
||
595 | */ |
||
596 | 2 | public function setYear($year) |
|
603 | |||
604 | /** |
||
605 | * @return string |
||
606 | */ |
||
607 | public function getTagged() |
||
613 | |||
614 | /** |
||
615 | * @param bool $tagged |
||
616 | * @return Media |
||
617 | */ |
||
618 | public function setTagged($tagged) |
||
625 | |||
626 | } |
||
627 |
Our type inference engine has found a suspicous assignment of a value to a property. This check raises an issue when a value that can be of a mixed type is assigned to a property that is type hinted more strictly.
For example, imagine you have a variable
$accountId
that can either hold an Id object or false (if there is no account id yet). Your code now assigns that value to theid
property of an instance of theAccount
class. This class holds a proper account, so the id value must no longer be false.Either this assignment is in error or a type check should be added for that assignment.