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 CdbXMLImporter 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 CdbXMLImporter, and based on these observations, apply Extract Interface, too.
| 1 | <?php  | 
            ||
| 23 | class CdbXMLImporter  | 
            ||
| 24 | { | 
            ||
| 25 | /**  | 
            ||
| 26 | * @var CdbXMLItemBaseImporter  | 
            ||
| 27 | */  | 
            ||
| 28 | private $cdbXMLItemBaseImporter;  | 
            ||
| 29 | |||
| 30 | /**  | 
            ||
| 31 | * @var EventCdbIdExtractorInterface  | 
            ||
| 32 | */  | 
            ||
| 33 | private $cdbIdExtractor;  | 
            ||
| 34 | |||
| 35 | /**  | 
            ||
| 36 | * @var PriceDescriptionParser  | 
            ||
| 37 | */  | 
            ||
| 38 | private $priceDescriptionParser;  | 
            ||
| 39 | |||
| 40 | /**  | 
            ||
| 41 | * @var StringFilterInterface  | 
            ||
| 42 | */  | 
            ||
| 43 | private $longDescriptionFilter;  | 
            ||
| 44 | |||
| 45 | /**  | 
            ||
| 46 | * @var StringFilterInterface  | 
            ||
| 47 | */  | 
            ||
| 48 | private $shortDescriptionFilter;  | 
            ||
| 49 | |||
| 50 | /**  | 
            ||
| 51 | * @var CalendarFactoryInterface  | 
            ||
| 52 | */  | 
            ||
| 53 | private $calendarFactory;  | 
            ||
| 54 | |||
| 55 | /**  | 
            ||
| 56 | * @var CdbXmlContactInfoImporterInterface  | 
            ||
| 57 | */  | 
            ||
| 58 | private $cdbXmlContactInfoImporter;  | 
            ||
| 59 | |||
| 60 | /**  | 
            ||
| 61 | * @param CdbXMLItemBaseImporter $cdbXMLItemBaseImporter  | 
            ||
| 62 | * @param EventCdbIdExtractorInterface $cdbIdExtractor  | 
            ||
| 63 | * @param PriceDescriptionParser $priceDescriptionParser  | 
            ||
| 64 | * @param CalendarFactoryInterface $calendarFactory  | 
            ||
| 65 | * @param CdbXmlContactInfoImporterInterface $cdbXmlContactInfoImporter  | 
            ||
| 66 | */  | 
            ||
| 67 | public function __construct(  | 
            ||
| 68 | CdbXMLItemBaseImporter $cdbXMLItemBaseImporter,  | 
            ||
| 69 | EventCdbIdExtractorInterface $cdbIdExtractor,  | 
            ||
| 70 | PriceDescriptionParser $priceDescriptionParser,  | 
            ||
| 71 | CalendarFactoryInterface $calendarFactory,  | 
            ||
| 72 | CdbXmlContactInfoImporterInterface $cdbXmlContactInfoImporter  | 
            ||
| 73 |     ) { | 
            ||
| 74 | $this->cdbXMLItemBaseImporter = $cdbXMLItemBaseImporter;  | 
            ||
| 75 | $this->cdbIdExtractor = $cdbIdExtractor;  | 
            ||
| 76 | $this->priceDescriptionParser = $priceDescriptionParser;  | 
            ||
| 77 | $this->calendarFactory = $calendarFactory;  | 
            ||
| 78 | $this->cdbXmlContactInfoImporter = $cdbXmlContactInfoImporter;  | 
            ||
| 79 | |||
| 80 | $consecutiveBlockOfTextFilter = new ConsecutiveBlockOfTextStringFilter();  | 
            ||
| 81 | |||
| 82 | $this->longDescriptionFilter = new CombinedStringFilter();  | 
            ||
| 83 | $this->longDescriptionFilter->addFilter(  | 
            ||
| 84 | new StripSourceStringFilter()  | 
            ||
| 85 | );  | 
            ||
| 86 | $this->longDescriptionFilter->addFilter(  | 
            ||
| 87 | $consecutiveBlockOfTextFilter  | 
            ||
| 88 | );  | 
            ||
| 89 | $this->longDescriptionFilter->addFilter(  | 
            ||
| 90 | new BreakTagToNewlineStringFilter()  | 
            ||
| 91 | );  | 
            ||
| 92 | $this->longDescriptionFilter->addFilter(  | 
            ||
| 93 | new StripSurroundingSpaceStringFilter()  | 
            ||
| 94 | );  | 
            ||
| 95 | |||
| 96 | $this->shortDescriptionFilter = $consecutiveBlockOfTextFilter;  | 
            ||
| 97 | }  | 
            ||
| 98 | |||
| 99 | /**  | 
            ||
| 100 | * Imports a UDB2 event into a UDB3 JSON-LD document.  | 
            ||
| 101 | *  | 
            ||
| 102 | * @param \stdClass $base  | 
            ||
| 103 | * The JSON-LD document to start from.  | 
            ||
| 104 | * @param \CultureFeed_Cdb_Item_Event $event  | 
            ||
| 105 | * The cultural event data from UDB2 to import.  | 
            ||
| 106 | * @param PlaceServiceInterface $placeManager  | 
            ||
| 107 | * The manager from which to retrieve the JSON-LD of a place.  | 
            ||
| 108 | * @param OrganizerServiceInterface $organizerManager  | 
            ||
| 109 | * The manager from which to retrieve the JSON-LD of an organizer.  | 
            ||
| 110 | * @param SluggerInterface $slugger  | 
            ||
| 111 | * The slugger that's used to generate a sameAs reference.  | 
            ||
| 112 | *  | 
            ||
| 113 | * @return \stdClass  | 
            ||
| 114 | * The document with the UDB2 event data merged in.  | 
            ||
| 115 | */  | 
            ||
| 116 | public function documentWithCdbXML(  | 
            ||
| 117 | $base,  | 
            ||
| 118 | \CultureFeed_Cdb_Item_Event $event,  | 
            ||
| 119 | PlaceServiceInterface $placeManager,  | 
            ||
| 120 | OrganizerServiceInterface $organizerManager,  | 
            ||
| 121 | SluggerInterface $slugger  | 
            ||
| 122 |     ) { | 
            ||
| 123 | $jsonLD = clone $base;  | 
            ||
| 124 | |||
| 125 | /** @var \CultureFeed_Cdb_Data_EventDetail $detail */  | 
            ||
| 126 | $detail = null;  | 
            ||
| 127 | |||
| 128 | /** @var \CultureFeed_Cdb_Data_EventDetail[] $details */  | 
            ||
| 129 | $details = $event->getDetails();  | 
            ||
| 130 | |||
| 131 |         foreach ($details as $languageDetail) { | 
            ||
| 132 | $language = $languageDetail->getLanguage();  | 
            ||
| 133 | |||
| 134 | // The first language detail found will be used to retrieve  | 
            ||
| 135 | // properties from which in UDB3 are not any longer considered  | 
            ||
| 136 | // to be language specific.  | 
            ||
| 137 |             if (!$detail) { | 
            ||
| 138 | $detail = $languageDetail;  | 
            ||
| 139 | }  | 
            ||
| 140 | |||
| 141 | $jsonLD->name[$language] = $languageDetail->getTitle();  | 
            ||
| 142 | |||
| 143 | $this->importDescription($languageDetail, $jsonLD, $language);  | 
            ||
| 144 | }  | 
            ||
| 145 | |||
| 146 | $this->cdbXMLItemBaseImporter->importAvailable($event, $jsonLD);  | 
            ||
| 147 | |||
| 148 | $labelImporter = new LabelImporter();  | 
            ||
| 149 | $labelImporter->importLabels($event, $jsonLD);  | 
            ||
| 150 | |||
| 151 | $jsonLD->calendarSummary = $detail->getCalendarSummary();  | 
            ||
| 152 | |||
| 153 | $this->importLocation($event, $placeManager, $jsonLD);  | 
            ||
| 154 | |||
| 155 | $this->importOrganizer($event, $organizerManager, $jsonLD);  | 
            ||
| 156 | |||
| 157 |         if ($event->getContactInfo()) { | 
            ||
| 158 | $this->cdbXmlContactInfoImporter->importBookingInfo(  | 
            ||
| 159 | $jsonLD,  | 
            ||
| 160 | $event->getContactInfo(),  | 
            ||
| 161 | $detail->getPrice(),  | 
            ||
| 162 | $event->getBookingPeriod()  | 
            ||
| 163 | );  | 
            ||
| 164 | }  | 
            ||
| 165 | |||
| 166 | $this->importPriceInfo($detail, $jsonLD);  | 
            ||
| 167 | |||
| 168 | $this->importTerms($event, $jsonLD);  | 
            ||
| 169 | |||
| 170 | $this->cdbXMLItemBaseImporter->importPublicationInfo($event, $jsonLD);  | 
            ||
| 171 | |||
| 172 | $calendar = $this->calendarFactory->createFromCdbCalendar($event->getCalendar());  | 
            ||
| 173 | $jsonLD = (object)array_merge((array)$jsonLD, $calendar->toJsonLd());  | 
            ||
| 174 | |||
| 175 | $this->importTypicalAgeRange($event, $jsonLD);  | 
            ||
| 176 | |||
| 177 | $this->importPerformers($detail, $jsonLD);  | 
            ||
| 178 | |||
| 179 | $this->importLanguages($event, $jsonLD);  | 
            ||
| 180 | |||
| 181 | $this->importUitInVlaanderenReference($event, $slugger, $jsonLD);  | 
            ||
| 182 | |||
| 183 | $this->cdbXMLItemBaseImporter->importExternalId($event, $jsonLD);  | 
            ||
| 184 | |||
| 185 | $this->importSeeAlso($event, $jsonLD);  | 
            ||
| 186 | |||
| 187 |         if ($event->getContactInfo()) { | 
            ||
| 188 | $this->cdbXmlContactInfoImporter->importContactPoint(  | 
            ||
| 189 | $jsonLD,  | 
            ||
| 190 | $event->getContactInfo()  | 
            ||
| 191 | );  | 
            ||
| 192 | }  | 
            ||
| 193 | |||
| 194 | $this->cdbXMLItemBaseImporter->importWorkflowStatus($event, $jsonLD);  | 
            ||
| 195 | |||
| 196 | return $jsonLD;  | 
            ||
| 197 | }  | 
            ||
| 198 | |||
| 199 | /**  | 
            ||
| 200 | * @param \CultureFeed_Cdb_Data_EventDetail $languageDetail  | 
            ||
| 201 | * @param \stdClass $jsonLD  | 
            ||
| 202 | * @param string $language  | 
            ||
| 203 | */  | 
            ||
| 204 | private function importDescription($languageDetail, $jsonLD, $language)  | 
            ||
| 205 |     { | 
            ||
| 206 | $longDescription = $languageDetail->getLongDescription();  | 
            ||
| 207 | |||
| 208 |         if ($longDescription) { | 
            ||
| 209 | $longDescription = $this->longDescriptionFilter->filter(  | 
            ||
| 210 | $longDescription  | 
            ||
| 211 | );  | 
            ||
| 212 | }  | 
            ||
| 213 | |||
| 214 | $descriptions = [];  | 
            ||
| 215 | |||
| 216 | $shortDescription = $languageDetail->getShortDescription();  | 
            ||
| 217 |         if ($shortDescription) { | 
            ||
| 218 | $includeShortDescription = true;  | 
            ||
| 219 | |||
| 220 | $shortDescription = $this->shortDescriptionFilter->filter(  | 
            ||
| 221 | $shortDescription  | 
            ||
| 222 | );  | 
            ||
| 223 | |||
| 224 |             if ($longDescription) { | 
            ||
| 225 | $includeShortDescription =  | 
            ||
| 226 | !$this->longDescriptionStartsWithShortDescription(  | 
            ||
| 227 | $longDescription,  | 
            ||
| 228 | $shortDescription  | 
            ||
| 229 | );  | 
            ||
| 230 | }  | 
            ||
| 231 | |||
| 232 |             if ($includeShortDescription) { | 
            ||
| 233 | $descriptions[] = $shortDescription;  | 
            ||
| 234 | }  | 
            ||
| 235 | }  | 
            ||
| 236 | |||
| 237 |         if ($longDescription) { | 
            ||
| 238 | $descriptions[] = $longDescription;  | 
            ||
| 239 | }  | 
            ||
| 240 | |||
| 241 |         $description = implode("\n\n", $descriptions); | 
            ||
| 242 | |||
| 243 | $jsonLD->description[$language] = $description;  | 
            ||
| 244 | }  | 
            ||
| 245 | |||
| 246 | /**  | 
            ||
| 247 | * @param \CultureFeed_Cdb_Item_Event $event  | 
            ||
| 248 | * @param PlaceServiceInterface $placeManager  | 
            ||
| 249 | * @param \stdClass $jsonLD  | 
            ||
| 250 | */  | 
            ||
| 251 | private function importLocation(\CultureFeed_Cdb_Item_Event $event, PlaceServiceInterface $placeManager, $jsonLD)  | 
            ||
| 277 | |||
| 278 | /**  | 
            ||
| 279 | * @param \CultureFeed_Cdb_Item_Event $event  | 
            ||
| 280 | * @param OrganizerServiceInterface $organizerManager  | 
            ||
| 281 | * @param \stdClass $jsonLD  | 
            ||
| 282 | */  | 
            ||
| 283 | private function importOrganizer(  | 
            ||
| 322 | |||
| 323 | /**  | 
            ||
| 324 | * @param \CultureFeed_Cdb_Data_EventDetail $detail  | 
            ||
| 325 | * @param \stdClass $jsonLD  | 
            ||
| 326 | */  | 
            ||
| 327 | private function importPriceInfo(  | 
            ||
| 374 | |||
| 375 | /**  | 
            ||
| 376 | * @param \CultureFeed_Cdb_Item_Event $event  | 
            ||
| 377 | * @param \stdClass $jsonLD  | 
            ||
| 378 | */  | 
            ||
| 379 | private function importTerms(\CultureFeed_Cdb_Item_Event $event, $jsonLD)  | 
            ||
| 399 | |||
| 400 | /**  | 
            ||
| 401 | * @param \CultureFeed_Cdb_Item_Event $event  | 
            ||
| 402 | * @param \stdClass $jsonLD  | 
            ||
| 403 | */  | 
            ||
| 404 | private function importTypicalAgeRange(\CultureFeed_Cdb_Item_Event $event, $jsonLD)  | 
            ||
| 420 | |||
| 421 | /**  | 
            ||
| 422 | * @param \CultureFeed_Cdb_Data_EventDetail $detail  | 
            ||
| 423 | * @param \stdClass $jsonLD  | 
            ||
| 424 | */  | 
            ||
| 425 | private function importPerformers(\CultureFeed_Cdb_Data_EventDetail $detail, $jsonLD)  | 
            ||
| 439 | |||
| 440 | /**  | 
            ||
| 441 | * @param \CultureFeed_Cdb_Item_Event $event  | 
            ||
| 442 | * @param \stdClass $jsonLD  | 
            ||
| 443 | */  | 
            ||
| 444 | private function importLanguages(\CultureFeed_Cdb_Item_Event $event, $jsonLD)  | 
            ||
| 456 | |||
| 457 | /**  | 
            ||
| 458 | * @param \CultureFeed_Cdb_Item_Event $event  | 
            ||
| 459 | * @param \stdClass $jsonLD  | 
            ||
| 460 | */  | 
            ||
| 461 | private function importSeeAlso(  | 
            ||
| 482 | |||
| 483 | /**  | 
            ||
| 484 | * @param \CultureFeed_Cdb_Item_Event $event  | 
            ||
| 485 | * @param SluggerInterface $slugger  | 
            ||
| 486 | * @param \stdClass $jsonLD  | 
            ||
| 487 | */  | 
            ||
| 488 | private function importUitInVlaanderenReference(  | 
            ||
| 515 | |||
| 516 | /**  | 
            ||
| 517 | * @param string $longDescription  | 
            ||
| 518 | * @param string $shortDescription  | 
            ||
| 519 | * @return bool  | 
            ||
| 520 | */  | 
            ||
| 521 | private function longDescriptionStartsWithShortDescription(  | 
            ||
| 529 | }  | 
            ||
| 530 | 
In PHP, under loose comparison (like
==, or!=, orswitchconditions), values of different types might be equal.For
stringvalues, the empty string''is a special case, in particular the following results might be unexpected: