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 Place 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 Place, and based on these observations, apply Extract Interface, too.
1 | <?php |
||
70 | class Place extends Offer implements UpdateableWithCdbXmlInterface |
||
71 | { |
||
72 | /** |
||
73 | * @var string |
||
74 | */ |
||
75 | private $placeId; |
||
76 | |||
77 | /** |
||
78 | * @var Address[] |
||
79 | */ |
||
80 | private $addresses; |
||
81 | |||
82 | /** |
||
83 | * @var boolean |
||
84 | */ |
||
85 | private $isDuplicate = false; |
||
86 | |||
87 | /** |
||
88 | * @var string[] |
||
89 | */ |
||
90 | private $duplicates = []; |
||
91 | |||
92 | public function __construct() |
||
93 | { |
||
94 | parent::__construct(); |
||
95 | |||
96 | $this->addresses = []; |
||
97 | } |
||
98 | |||
99 | /** |
||
100 | * {@inheritdoc} |
||
101 | */ |
||
102 | public function getAggregateRootId() |
||
103 | { |
||
104 | return $this->placeId; |
||
105 | } |
||
106 | |||
107 | /** |
||
108 | * @todo Rename this method to create() after moving this part of the codebase to udb3-silex |
||
109 | */ |
||
110 | public static function createPlace( |
||
111 | string $id, |
||
112 | Language $mainLanguage, |
||
113 | Title $title, |
||
114 | EventType $eventType, |
||
115 | Address $address, |
||
116 | CalendarInterface $calendar, |
||
117 | Theme $theme = null, |
||
118 | DateTimeImmutable $publicationDate = null |
||
119 | ): self { |
||
120 | $place = new self(); |
||
121 | $place->apply(new PlaceCreated( |
||
122 | $id, |
||
123 | $mainLanguage, |
||
124 | $title, |
||
125 | $eventType, |
||
126 | $address, |
||
127 | $calendar, |
||
128 | $theme, |
||
129 | $publicationDate |
||
130 | )); |
||
131 | |||
132 | return $place; |
||
133 | } |
||
134 | |||
135 | View Code Duplication | protected function applyPlaceCreated(PlaceCreated $placeCreated): void |
|
|
|||
136 | { |
||
137 | $this->mainLanguage = $placeCreated->getMainLanguage(); |
||
138 | $this->titles[$this->mainLanguage->getCode()] = $placeCreated->getTitle(); |
||
139 | $this->calendar = $placeCreated->getCalendar(); |
||
140 | $this->contactPoint = new ContactPoint(); |
||
141 | $this->bookingInfo = new BookingInfo(); |
||
142 | $this->typeId = $placeCreated->getEventType()->getId(); |
||
143 | $this->themeId = $placeCreated->getTheme() ? $placeCreated->getTheme()->getId() : null; |
||
144 | $this->addresses[$this->mainLanguage->getCode()] = $placeCreated->getAddress(); |
||
145 | $this->placeId = $placeCreated->getPlaceId(); |
||
146 | $this->workflowStatus = WorkflowStatus::DRAFT(); |
||
147 | } |
||
148 | |||
149 | public function updateMajorInfo( |
||
150 | Title $title, |
||
151 | EventType $eventType, |
||
152 | Address $address, |
||
153 | CalendarInterface $calendar, |
||
154 | Theme $theme = null |
||
155 | ): void { |
||
156 | $this->apply( |
||
157 | new MajorInfoUpdated( |
||
158 | $this->placeId, |
||
159 | $title, |
||
160 | $eventType, |
||
161 | $address, |
||
162 | $calendar, |
||
163 | $theme |
||
164 | ) |
||
165 | ); |
||
166 | } |
||
167 | |||
168 | protected function applyMajorInfoUpdated(MajorInfoUpdated $majorInfoUpdated): void |
||
169 | { |
||
170 | $this->addresses[$this->mainLanguage->getCode()] = $majorInfoUpdated->getAddress(); |
||
171 | } |
||
172 | |||
173 | View Code Duplication | public function updateAddress(Address $address, Language $language): void |
|
174 | { |
||
175 | if ($language->getCode() === $this->mainLanguage->getCode()) { |
||
176 | $event = new AddressUpdated($this->placeId, $address); |
||
177 | } else { |
||
178 | $event = new AddressTranslated($this->placeId, $address, $language); |
||
179 | } |
||
180 | |||
181 | if ($this->allowAddressUpdate($address, $language)) { |
||
182 | $this->apply($event); |
||
183 | } |
||
184 | } |
||
185 | |||
186 | protected function applyAddressUpdated(AddressUpdated $addressUpdated): void |
||
187 | { |
||
188 | $this->addresses[$this->mainLanguage->getCode()] = $addressUpdated->getAddress(); |
||
189 | } |
||
190 | |||
191 | protected function applyAddressTranslated(AddressTranslated $addressTranslated): void |
||
192 | { |
||
193 | $this->addresses[$addressTranslated->getLanguage()->getCode()] = $addressTranslated->getAddress(); |
||
194 | } |
||
195 | |||
196 | View Code Duplication | public function markAsDuplicateOf(string $placeIdOfCanonical): void |
|
197 | { |
||
198 | if ($this->isDeleted) { |
||
199 | throw CannotMarkPlaceAsDuplicate::becauseItIsDeleted($this->placeId); |
||
200 | } |
||
201 | |||
202 | if ($this->isDuplicate) { |
||
203 | throw CannotMarkPlaceAsDuplicate::becauseItIsAlreadyADuplicate($this->placeId); |
||
204 | } |
||
205 | |||
206 | $this->apply(new MarkedAsDuplicate($this->placeId, $placeIdOfCanonical)); |
||
207 | } |
||
208 | |||
209 | View Code Duplication | public function markAsCanonicalFor(string $placeIdOfDuplicate, array $duplicatesOfDuplicate = []): void |
|
210 | { |
||
211 | if ($this->isDeleted) { |
||
212 | throw CannotMarkPlaceAsCanonical::becauseItIsDeleted($this->placeId); |
||
213 | } |
||
214 | |||
215 | if ($this->isDuplicate) { |
||
216 | throw CannotMarkPlaceAsCanonical::becauseItIsAlreadyADuplicate($this->placeId); |
||
217 | } |
||
218 | |||
219 | $this->apply(new MarkedAsCanonical($this->placeId, $placeIdOfDuplicate, $duplicatesOfDuplicate)); |
||
220 | } |
||
221 | |||
222 | /** |
||
223 | * @return string[] |
||
224 | */ |
||
225 | public function getDuplicates(): array |
||
226 | { |
||
227 | return $this->duplicates; |
||
228 | } |
||
229 | |||
230 | private function allowAddressUpdate(Address $address, Language $language): bool |
||
244 | |||
245 | public static function importFromUDB2Actor( |
||
246 | string $actorId, |
||
247 | string $cdbXml, |
||
248 | string $cdbXmlNamespaceUri |
||
249 | ): self { |
||
250 | $place = new static(); |
||
251 | $place->apply( |
||
252 | new PlaceImportedFromUDB2( |
||
253 | $actorId, |
||
254 | $cdbXml, |
||
255 | $cdbXmlNamespaceUri |
||
256 | ) |
||
261 | |||
262 | protected function applyPlaceImportedFromUDB2(PlaceImportedFromUDB2 $placeImported): void |
||
295 | |||
296 | protected function applyPlaceUpdatedFromUDB2(PlaceUpdatedFromUDB2 $placeUpdatedFromUDB2): void |
||
328 | |||
329 | protected function applyPlaceDeleted(PlaceDeleted $event): void |
||
333 | |||
334 | protected function applyMarkedAsDuplicate(MarkedAsDuplicate $event): void |
||
338 | |||
339 | protected function applyMarkedAsCanonical(MarkedAsCanonical $event): void |
||
346 | |||
347 | /** |
||
348 | * @inheritdoc |
||
349 | */ |
||
350 | public function updateWithCdbXml($cdbXml, $cdbXmlNamespaceUri) |
||
362 | |||
363 | /** |
||
364 | * @param Label $label |
||
365 | * @return LabelAdded |
||
366 | */ |
||
367 | protected function createLabelAddedEvent(Label $label) |
||
371 | |||
372 | /** |
||
373 | * @param Label $label |
||
374 | * @return LabelRemoved |
||
375 | */ |
||
376 | protected function createLabelRemovedEvent(Label $label) |
||
380 | |||
381 | /** |
||
382 | * @inheritdoc |
||
383 | */ |
||
384 | protected function createLabelsImportedEvent(Labels $labels) |
||
388 | |||
389 | protected function createImageAddedEvent(Image $image) |
||
393 | |||
394 | protected function createImageRemovedEvent(Image $image) |
||
398 | |||
399 | protected function createImageUpdatedEvent( |
||
411 | |||
412 | protected function createMainImageSelectedEvent(Image $image) |
||
416 | |||
417 | /** |
||
418 | * @inheritdoc |
||
419 | */ |
||
420 | protected function createTitleTranslatedEvent(Language $language, Title $title) |
||
424 | |||
425 | /** |
||
426 | * @param Title $title |
||
427 | * @return TitleUpdated |
||
428 | */ |
||
429 | protected function createTitleUpdatedEvent(Title $title) |
||
433 | |||
434 | /** |
||
435 | * @inheritdoc |
||
436 | */ |
||
437 | protected function createDescriptionTranslatedEvent(Language $language, Description $description) |
||
441 | |||
442 | /** |
||
443 | * @inheritdoc |
||
444 | */ |
||
445 | protected function createDescriptionUpdatedEvent(Description $description) |
||
449 | |||
450 | /** |
||
451 | * @inheritdoc |
||
452 | */ |
||
453 | protected function createCalendarUpdatedEvent(Calendar $calendar) |
||
457 | |||
458 | /** |
||
459 | * @param AgeRange $typicalAgeRange |
||
460 | * @return TypicalAgeRangeUpdated |
||
461 | */ |
||
462 | protected function createTypicalAgeRangeUpdatedEvent($typicalAgeRange) |
||
466 | |||
467 | /** |
||
468 | * @return TypicalAgeRangeDeleted |
||
469 | */ |
||
470 | protected function createTypicalAgeRangeDeletedEvent() |
||
474 | |||
475 | /** |
||
476 | * @param string $organizerId |
||
477 | * @return OrganizerUpdated |
||
478 | */ |
||
479 | protected function createOrganizerUpdatedEvent($organizerId) |
||
483 | |||
484 | /** |
||
485 | * @param string $organizerId |
||
486 | * @return OrganizerDeleted |
||
487 | */ |
||
488 | protected function createOrganizerDeletedEvent($organizerId) |
||
492 | |||
493 | /** |
||
494 | * @param ContactPoint $contactPoint |
||
495 | * @return ContactPointUpdated |
||
496 | */ |
||
497 | protected function createContactPointUpdatedEvent(ContactPoint $contactPoint) |
||
501 | |||
502 | /** |
||
503 | * @inheritdoc |
||
504 | */ |
||
505 | protected function createGeoCoordinatesUpdatedEvent(Coordinates $coordinates) |
||
509 | |||
510 | /** |
||
511 | * @param BookingInfo $bookingInfo |
||
512 | * @return BookingInfoUpdated |
||
513 | */ |
||
514 | protected function createBookingInfoUpdatedEvent(BookingInfo $bookingInfo) |
||
518 | |||
519 | /** |
||
520 | * @param PriceInfo $priceInfo |
||
521 | * @return PriceInfoUpdated |
||
522 | */ |
||
523 | protected function createPriceInfoUpdatedEvent(PriceInfo $priceInfo) |
||
527 | |||
528 | /** |
||
529 | * @return PlaceDeleted |
||
530 | */ |
||
531 | protected function createOfferDeletedEvent() |
||
535 | |||
536 | /** |
||
537 | * @inheritDoc |
||
538 | */ |
||
539 | protected function createPublishedEvent(\DateTimeInterface $publicationDate) |
||
543 | |||
544 | /** |
||
545 | * @inheritDoc |
||
546 | */ |
||
547 | protected function createApprovedEvent() |
||
551 | |||
552 | /** |
||
553 | * @inheritDoc |
||
554 | */ |
||
555 | protected function createRejectedEvent(StringLiteral $reason) |
||
559 | |||
560 | /** |
||
561 | * @inheritDoc |
||
562 | */ |
||
563 | protected function createFlaggedAsDuplicate() |
||
567 | |||
568 | /** |
||
569 | * @inheritDoc |
||
570 | */ |
||
571 | protected function createFlaggedAsInappropriate() |
||
575 | |||
576 | /** |
||
577 | * @inheritDoc |
||
578 | * @return ImagesImportedFromUDB2 |
||
579 | */ |
||
580 | protected function createImagesImportedFromUDB2(ImageCollection $images) |
||
584 | |||
585 | /** |
||
586 | * @inheritDoc |
||
587 | * @return ImagesUpdatedFromUDB2 |
||
588 | */ |
||
589 | protected function createImagesUpdatedFromUDB2(ImageCollection $images) |
||
593 | |||
594 | protected function createTypeUpdatedEvent(EventType $type) |
||
598 | |||
599 | protected function createThemeUpdatedEvent(Theme $theme) |
||
603 | |||
604 | /** |
||
605 | * @inheritdoc |
||
606 | */ |
||
607 | protected function createFacilitiesUpdatedEvent(array $facilities) |
||
611 | } |
||
612 |
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.