eBayEnterprise /
RetailOrderManagement-SDK
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 OrderDetailItem 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 OrderDetailItem, and based on these observations, apply Extract Interface, too.
| 1 | <?php |
||
| 32 | class OrderDetailItem extends OrderItem implements IOrderDetailItem |
||
| 33 | { |
||
| 34 | use TStatusContainer, TChargeGroupContainer; |
||
| 35 | |||
| 36 | /** @var bool */ |
||
| 37 | protected $hasChainedLines; |
||
| 38 | /** @var bool */ |
||
| 39 | protected $hasDerivedChild; |
||
| 40 | /** @var string */ |
||
| 41 | protected $chainedFromOrderHeaderKey; |
||
| 42 | /** @var string */ |
||
| 43 | protected $derivedFromOrderHeaderKey; |
||
| 44 | /** @var float */ |
||
| 45 | protected $shippedQuantity; |
||
| 46 | /** @var string */ |
||
| 47 | protected $carrier; |
||
| 48 | /** @var string */ |
||
| 49 | protected $carrierMode; |
||
| 50 | /** @var string */ |
||
| 51 | protected $carrierDisplayText; |
||
| 52 | /** @var DateTime */ |
||
| 53 | protected $originalExpectedShipmentDateFrom; |
||
| 54 | /** @var DateTime */ |
||
| 55 | protected $originalExpectedShipmentDateTo; |
||
| 56 | /** @var DateTime */ |
||
| 57 | protected $originalExpectedDeliveryDateFrom; |
||
| 58 | /** @var DateTime */ |
||
| 59 | protected $originalExpectedDeliveryDateTo; |
||
| 60 | /** @var string */ |
||
| 61 | protected $omsLineId; |
||
| 62 | /** @var ICustomerCareOrderItemTotals */ |
||
| 63 | protected $customerCareOrderItemTotals; |
||
| 64 | |||
| 65 | /** |
||
| 66 | * @param IValidatorIterator |
||
| 67 | * @param ISchemaValidator |
||
| 68 | * @param IPayloadMap |
||
| 69 | * @param LoggerInterface |
||
| 70 | * @param IPayload |
||
| 71 | * @SuppressWarnings(PHPMD.UnusedFormalParameter) |
||
| 72 | */ |
||
| 73 | View Code Duplication | public function __construct( |
|
|
|
|||
| 74 | IValidatorIterator $validators, |
||
| 75 | ISchemaValidator $schemaValidator, |
||
| 76 | IPayloadMap $payloadMap, |
||
| 77 | LoggerInterface $logger, |
||
| 78 | IPayload $parentPayload = null |
||
| 79 | ) { |
||
| 80 | parent::__construct($validators, $schemaValidator, $payloadMap, $logger, $parentPayload); |
||
| 81 | $this->initOptionalExtractPaths() |
||
| 82 | ->initDatetimeExtractPaths() |
||
| 83 | ->initSubPayloadExtractPaths() |
||
| 84 | ->initSubPayloadProperties(); |
||
| 85 | } |
||
| 86 | |||
| 87 | /** |
||
| 88 | * Initialize the protected class property array self::optionalExtractionPaths with xpath |
||
| 89 | * key/value pairs. |
||
| 90 | * |
||
| 91 | * @return self |
||
| 92 | */ |
||
| 93 | protected function initOptionalExtractPaths() |
||
| 94 | { |
||
| 95 | $this->optionalExtractionPaths = array_merge($this->optionalExtractionPaths, [ |
||
| 96 | 'chainedFromOrderHeaderKey' => '@chainedFromOrderHeaderKey', |
||
| 97 | 'derivedFromOrderHeaderKey' => '@derivedFromOrderHeaderKey', |
||
| 98 | 'hasChainedLines' => '@hasChainedLines', |
||
| 99 | 'hasDerivedChild' => '@hasDerivedChild', |
||
| 100 | 'shippedQuantity' => 'x:ShippedQuantity', |
||
| 101 | 'carrier' => 'x:Carrier', |
||
| 102 | 'carrierMode' => 'x:Carrier/@mode', |
||
| 103 | 'carrierDisplayText' => 'x:Carrier/@displayText', |
||
| 104 | 'omsLineId' => 'x:OMSLineId', |
||
| 105 | ]); |
||
| 106 | return $this; |
||
| 107 | } |
||
| 108 | |||
| 109 | /** |
||
| 110 | * Initialize the protected class property array self::datetimeExtractionPaths with xpath |
||
| 111 | * key/value pairs. |
||
| 112 | * |
||
| 113 | * @return self |
||
| 114 | */ |
||
| 115 | protected function initDatetimeExtractPaths() |
||
| 116 | { |
||
| 117 | $this->datetimeExtractionPaths = array_merge($this->datetimeExtractionPaths, [ |
||
| 118 | 'originalExpectedShipmentDateFrom' => 'string(x:EstimatedDeliveryDate/x:OriginalExpectedShipmentDate/x:From)', |
||
| 119 | 'originalExpectedShipmentDateTo' => 'string(x:EstimatedDeliveryDate/x:OriginalExpectedShipmentDate/x:To)', |
||
| 120 | 'originalExpectedDeliveryDateFrom' => 'string(x:EstimatedDeliveryDate/x:OriginalExpectedDeliveryDate/x:From)', |
||
| 121 | 'originalExpectedDeliveryDateTo' => 'string(x:EstimatedDeliveryDate/x:OriginalExpectedDeliveryDate/x:To)', |
||
| 122 | ]); |
||
| 123 | return $this; |
||
| 124 | } |
||
| 125 | |||
| 126 | /** |
||
| 127 | * Initialize the protected class property array self::subpayloadExtractionPaths with xpath |
||
| 128 | * key/value pairs. |
||
| 129 | * |
||
| 130 | * @return self |
||
| 131 | */ |
||
| 132 | protected function initSubPayloadExtractPaths() |
||
| 133 | { |
||
| 134 | $this->subpayloadExtractionPaths = array_merge($this->subpayloadExtractionPaths, [ |
||
| 135 | 'statuses' => 'x:Statuses', |
||
| 136 | 'customerCareOrderItemTotals' => 'x:CustomerCareOrderItemTotals', |
||
| 137 | 'chargeGroups' => 'x:ChargeGroups', |
||
| 138 | ]); |
||
| 139 | return $this; |
||
| 140 | } |
||
| 141 | |||
| 142 | /** |
||
| 143 | * Initialize any sub-payload class properties with their concrete instance. |
||
| 144 | * |
||
| 145 | * @return self |
||
| 146 | */ |
||
| 147 | protected function initSubPayloadProperties() |
||
| 148 | { |
||
| 149 | $this->setStatuses($this->buildPayloadForInterface( |
||
| 150 | self::STATUS_ITERABLE_INTERFACE |
||
| 151 | )); |
||
| 152 | $this->setCustomerCareOrderItemTotals($this->buildPayloadForInterface( |
||
| 153 | self::CUSTOMER_CARE_ORDER_ITEM_TOTALS_INTERFACE |
||
| 154 | )); |
||
| 155 | $this->setChargeGroups($this->buildPayloadForInterface( |
||
| 156 | self::CHARGE_GROUP_ITERABLE_INTERFACE |
||
| 157 | )); |
||
| 158 | return $this; |
||
| 159 | } |
||
| 160 | |||
| 161 | /** |
||
| 162 | * @see IOrderDetailItem::getHasChainedLines() |
||
| 163 | */ |
||
| 164 | public function getHasChainedLines() |
||
| 168 | |||
| 169 | /** |
||
| 170 | * @see IOrderDetailItem::setCustomerOrderId() |
||
| 171 | * @codeCoverageIgnore |
||
| 172 | */ |
||
| 173 | public function setHasChainedLines($hasChainedLines) |
||
| 174 | { |
||
| 175 | $this->hasChainedLines = $hasChainedLines; |
||
| 176 | return $this; |
||
| 177 | } |
||
| 178 | |||
| 179 | /** |
||
| 180 | * @see IOrderDetailItem::getHasDerivedChild() |
||
| 181 | */ |
||
| 182 | public function getHasDerivedChild() |
||
| 186 | |||
| 187 | /** |
||
| 188 | * @see IOrderDetailItem::setCustomerOrderId() |
||
| 189 | * @codeCoverageIgnore |
||
| 190 | */ |
||
| 191 | public function setHasDerivedChild($hasDerivedChild) |
||
| 192 | { |
||
| 193 | $this->hasDerivedChild = $hasDerivedChild; |
||
| 194 | return $this; |
||
| 195 | } |
||
| 196 | |||
| 197 | /** |
||
| 198 | * @see IOrderDetailItem::getChainedFromOrderHeaderKey() |
||
| 199 | */ |
||
| 200 | public function getChainedFromOrderHeaderKey() |
||
| 204 | |||
| 205 | /** |
||
| 206 | * @see IOrderDetailItem::setCustomerOrderId() |
||
| 207 | * @codeCoverageIgnore |
||
| 208 | */ |
||
| 209 | public function setChainedFromOrderHeaderKey($chainedFromOrderHeaderKey) |
||
| 210 | { |
||
| 211 | $this->chainedFromOrderHeaderKey = $chainedFromOrderHeaderKey; |
||
| 212 | return $this; |
||
| 213 | } |
||
| 214 | |||
| 215 | /** |
||
| 216 | * @see IOrderDetailItem::getDerivedFromOrderHeaderKey() |
||
| 217 | */ |
||
| 218 | public function getDerivedFromOrderHeaderKey() |
||
| 222 | |||
| 223 | /** |
||
| 224 | * @see IOrderDetailItem::setCustomerOrderId() |
||
| 225 | * @codeCoverageIgnore |
||
| 226 | */ |
||
| 227 | public function setDerivedFromOrderHeaderKey($derivedFromOrderHeaderKey) |
||
| 228 | { |
||
| 229 | $this->derivedFromOrderHeaderKey = $derivedFromOrderHeaderKey; |
||
| 230 | return $this; |
||
| 231 | } |
||
| 232 | |||
| 233 | /** |
||
| 234 | * @see IOrderDetailItem::getShippedQuantity() |
||
| 235 | */ |
||
| 236 | public function getShippedQuantity() |
||
| 240 | |||
| 241 | /** |
||
| 242 | * @see IOrderDetailItem::setShippedQuantity() |
||
| 243 | * @codeCoverageIgnore |
||
| 244 | */ |
||
| 245 | public function setShippedQuantity($shippedQuantity) |
||
| 246 | { |
||
| 247 | $this->shippedQuantity = $shippedQuantity; |
||
| 248 | return $this; |
||
| 249 | } |
||
| 250 | |||
| 251 | /** |
||
| 252 | * @see IOrderDetailItem::getCarrier() |
||
| 253 | */ |
||
| 254 | public function getCarrier() |
||
| 258 | |||
| 259 | /** |
||
| 260 | * @see IOrderDetailItem::setCarrier() |
||
| 261 | * @codeCoverageIgnore |
||
| 262 | */ |
||
| 263 | public function setCarrier($carrier) |
||
| 264 | { |
||
| 265 | $this->carrier = $carrier; |
||
| 266 | return $this; |
||
| 267 | } |
||
| 268 | |||
| 269 | /** |
||
| 270 | * @see IOrderDetailItem::getCarrierMode() |
||
| 271 | */ |
||
| 272 | public function getCarrierMode() |
||
| 276 | |||
| 277 | /** |
||
| 278 | * @see IOrderDetailItem::setCarrierMode() |
||
| 279 | * @codeCoverageIgnore |
||
| 280 | */ |
||
| 281 | public function setCarrierMode($carrierMode) |
||
| 282 | { |
||
| 283 | $this->carrierMode = $carrierMode; |
||
| 284 | return $this; |
||
| 285 | } |
||
| 286 | |||
| 287 | /** |
||
| 288 | * @see IOrderDetailItem::getCarrierDisplayText() |
||
| 289 | */ |
||
| 290 | public function getCarrierDisplayText() |
||
| 294 | |||
| 295 | /** |
||
| 296 | * @see IOrderDetailItem::setCarrierDisplayText() |
||
| 297 | * @codeCoverageIgnore |
||
| 298 | */ |
||
| 299 | public function setCarrierDisplayText($carrierDisplayText) |
||
| 300 | { |
||
| 301 | $this->carrierDisplayText = $carrierDisplayText; |
||
| 302 | return $this; |
||
| 303 | } |
||
| 304 | |||
| 305 | /** |
||
| 306 | * @see IOrderDetailItem::getOriginalExpectedShipmentDateFrom() |
||
| 307 | */ |
||
| 308 | public function getOriginalExpectedShipmentDateFrom() |
||
| 312 | |||
| 313 | /** |
||
| 314 | * @see IOrderDetailItem::setOriginalExpectedShipmentDateFrom() |
||
| 315 | * @codeCoverageIgnore |
||
| 316 | */ |
||
| 317 | public function setOriginalExpectedShipmentDateFrom(DateTime $originalExpectedShipmentDateFrom) |
||
| 318 | { |
||
| 319 | $this->originalExpectedShipmentDateFrom = $originalExpectedShipmentDateFrom; |
||
| 320 | return $this; |
||
| 321 | } |
||
| 322 | |||
| 323 | /** |
||
| 324 | * @see IOrderDetailItem::getOriginalExpectedShipmentDateTo() |
||
| 325 | */ |
||
| 326 | public function getOriginalExpectedShipmentDateTo() |
||
| 330 | |||
| 331 | /** |
||
| 332 | * @see IOrderDetailItem::setOriginalExpectedShipmentDateTo() |
||
| 333 | * @codeCoverageIgnore |
||
| 334 | */ |
||
| 335 | public function setOriginalExpectedShipmentDateTo(DateTime $originalExpectedShipmentDateTo) |
||
| 336 | { |
||
| 337 | $this->originalExpectedShipmentDateTo = $originalExpectedShipmentDateTo; |
||
| 338 | return $this; |
||
| 339 | } |
||
| 340 | |||
| 341 | /** |
||
| 342 | * @see IOrderDetailItem::getOriginalExpectedDeliveryDateFrom() |
||
| 343 | */ |
||
| 344 | public function getOriginalExpectedDeliveryDateFrom() |
||
| 348 | |||
| 349 | /** |
||
| 350 | * @see IOrderDetailItem::setOriginalExpectedDeliveryDateFrom() |
||
| 351 | * @codeCoverageIgnore |
||
| 352 | */ |
||
| 353 | public function setOriginalExpectedDeliveryDateFrom(DateTime $originalExpectedDeliveryDateFrom) |
||
| 354 | { |
||
| 355 | $this->originalExpectedDeliveryDateFrom = $originalExpectedDeliveryDateFrom; |
||
| 356 | return $this; |
||
| 357 | } |
||
| 358 | |||
| 359 | /** |
||
| 360 | * @see IOrderDetailItem::getOriginalExpectedDeliveryDateTo() |
||
| 361 | */ |
||
| 362 | public function getOriginalExpectedDeliveryDateTo() |
||
| 366 | |||
| 367 | /** |
||
| 368 | * @see IOrderDetailItem::setOriginalExpectedDeliveryDateTo() |
||
| 369 | * @codeCoverageIgnore |
||
| 370 | */ |
||
| 371 | public function setOriginalExpectedDeliveryDateTo(DateTime $originalExpectedDeliveryDateTo) |
||
| 372 | { |
||
| 373 | $this->originalExpectedDeliveryDateTo = $originalExpectedDeliveryDateTo; |
||
| 374 | return $this; |
||
| 375 | } |
||
| 376 | |||
| 377 | /** |
||
| 378 | * @see IOrderDetailItem::getOmsLineId() |
||
| 379 | */ |
||
| 380 | public function getOmsLineId() |
||
| 384 | |||
| 385 | /** |
||
| 386 | * @see IOrderDetailItem::setOmsLineId() |
||
| 387 | * @codeCoverageIgnore |
||
| 388 | */ |
||
| 389 | public function setOmsLineId($omsLineId) |
||
| 390 | { |
||
| 391 | $this->omsLineId = $omsLineId; |
||
| 392 | return $this; |
||
| 393 | } |
||
| 394 | |||
| 395 | /** |
||
| 396 | * @see IOrderDetailItem::getCustomerCareOrderItemTotals() |
||
| 397 | */ |
||
| 398 | public function getCustomerCareOrderItemTotals() |
||
| 402 | |||
| 403 | /** |
||
| 404 | * @see IOrderDetailItem::setCustomerCareOrderItemTotals() |
||
| 405 | * @codeCoverageIgnore |
||
| 406 | */ |
||
| 407 | public function setCustomerCareOrderItemTotals(ICustomerCareOrderItemTotals $customerCareOrderItemTotals) |
||
| 408 | { |
||
| 409 | $this->customerCareOrderItemTotals = $customerCareOrderItemTotals; |
||
| 410 | return $this; |
||
| 411 | } |
||
| 412 | |||
| 413 | View Code Duplication | protected function getRootAttributes() |
|
| 414 | { |
||
| 415 | $hasChainedLines = $this->getHasChainedLines(); |
||
| 416 | $hasDerivedChild = $this->getHasDerivedChild(); |
||
| 417 | $chainedFromOrderHeaderKey = $this->getChainedFromOrderHeaderKey(); |
||
| 418 | $derivedFromOrderHeaderKey = $this->getDerivedFromOrderHeaderKey(); |
||
| 419 | return array_merge( |
||
| 420 | parent::getRootAttributes(), |
||
| 421 | !empty($hasChainedLines) ? ['hasChainedLines' => $hasChainedLines] : [], |
||
| 427 | |||
| 428 | protected function serializeContents() |
||
| 459 | |||
| 460 | /** |
||
| 461 | * Create an XML serialization of the estimated deliver date data. Will |
||
| 462 | * only include any data when there is useful EDD data to include. |
||
| 463 | * |
||
| 464 | * @return string |
||
| 465 | */ |
||
| 466 | protected function serializeEstimatedDeliveryDate() |
||
| 497 | |||
| 498 | /** |
||
| 499 | * Serialize the carrier XML node. |
||
| 500 | * |
||
| 501 | * @param string |
||
| 502 | * @param string |
||
| 503 | * @return string | null |
||
| 504 | */ |
||
| 505 | View Code Duplication | protected function serializeCarrierValue($nodeName, $value) |
|
| 512 | } |
||
| 513 |
Loading history...
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.