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 EbayEnterprise_Catalog_Helper_Map 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 EbayEnterprise_Catalog_Helper_Map, and based on these observations, apply Extract Interface, too.
1 | <?php |
||
32 | class EbayEnterprise_Catalog_Helper_Map |
||
33 | { |
||
34 | const TYPE_GIFTCARD = 'giftcard'; |
||
35 | |||
36 | /** @var EbayEnterprise_MageLog_Helper_Data */ |
||
37 | protected $logger; |
||
38 | /** @var EbayEnterprise_MageLog_Helper_Context */ |
||
39 | protected $context; |
||
40 | /** @var EbayEnterprise_Eb2cCore_Helper_Data */ |
||
41 | protected $coreHelper; |
||
42 | /** @var EbayEnterprise_Catalog_Helper_Data */ |
||
43 | protected $catalogHelper; |
||
44 | |||
45 | /** |
||
46 | * Map ownerDocuments to DomXPath objects to avoid recreating them. |
||
47 | * |
||
48 | * @var SplObjectStorage |
||
49 | */ |
||
50 | protected $_splStorageDocMap = null; |
||
51 | /** |
||
52 | * Keep from having to reinstantiate this collection when doing the product imports. |
||
53 | * |
||
54 | * @var Mage_Catalog_Model_Resource_Category_Collection |
||
55 | */ |
||
56 | protected $_categoryCollection = null; |
||
57 | |||
58 | View Code Duplication | public function __construct(array $initParams = []) |
|
72 | |||
73 | /** |
||
74 | * Type hinting for self::__construct $initParams |
||
75 | * |
||
76 | * @param EbayEnterprise_Eb2cCore_Helper_Data |
||
77 | * @param EbayEnterprise_Catalog_Helper_Data |
||
78 | * @param EbayEnterprise_MageLog_Helper_Data |
||
79 | * @param EbayEnterprise_MageLog_Helper_Context |
||
80 | * @return array |
||
81 | */ |
||
82 | protected function checkTypes( |
||
91 | |||
92 | /** |
||
93 | * Return the value at field in array if it exists. Otherwise, use the default value. |
||
94 | * |
||
95 | * @param array |
||
96 | * @param string $field Valid array key |
||
97 | * @param mixed |
||
98 | * @return mixed |
||
99 | */ |
||
100 | protected function nullCoalesce(array $arr, $field, $default) |
||
104 | |||
105 | /** |
||
106 | * check if the node list has item and if the first item node value equal to 'active' to return |
||
107 | * the status for enable otherwise status for disable |
||
108 | * @param DOMNodeList $nodes |
||
109 | * @return string |
||
110 | */ |
||
111 | public function extractStatusValue(DOMNodeList $nodes) |
||
117 | /** |
||
118 | * extracts visibility from node list, returning an expected integer or translating an expected string |
||
119 | * into an expected integer, returning null for unexpected values |
||
120 | * @param DOMNodeList $nodes |
||
121 | * @return int|null |
||
122 | */ |
||
123 | public function extractVisibilityValue(DOMNodeList $nodes, Mage_Catalog_Model_Product $product) |
||
145 | /** |
||
146 | * extract the first element of a dom node list make sure it is lower case |
||
147 | * if there's no item in the DOMNodeList return the default simple product type constant value |
||
148 | * |
||
149 | * @param DOMNodeList $nodes |
||
150 | * @param Mage_Catalog_Model_Product $product |
||
151 | * @return string |
||
152 | */ |
||
153 | public function extractProductTypeValue(DOMNodeList $nodes, Mage_Catalog_Model_Product $product) |
||
161 | |||
162 | /** |
||
163 | * check if a given string is a valid product type value |
||
164 | * @param string $value the value that must match one of magento product type |
||
165 | * @return bool true the value match magento product type otherwise false |
||
166 | */ |
||
167 | protected function _isValidProductType($value) |
||
178 | |||
179 | /** |
||
180 | * This should produce a serialized array of product links to be handled by |
||
181 | * the product cleaner. Arrays should consist of |
||
182 | * |
||
183 | * @param DOMNodeList $nodes DOM nodes extracted from the feed |
||
184 | * @return string Serialized array |
||
185 | */ |
||
186 | public function extractProductLinks(DOMNodeList $nodes) |
||
209 | /** |
||
210 | * Convert the EB2C link types to link types Magento knows about through a |
||
211 | * mapping in the product config.xml. |
||
212 | * @param string $linkType |
||
213 | * @return string |
||
214 | * @throws Mage_Core_Exception If the link type is not mapped to a Magento link type |
||
215 | */ |
||
216 | protected function _convertToMagentoLinkType($linkType) |
||
220 | |||
221 | /** |
||
222 | * extract the value in the nodelist and then passed it to helper |
||
223 | * function to ensure the value has the right info |
||
224 | * @param DOMNodeList $nodes |
||
225 | * @return string |
||
226 | */ |
||
227 | public function extractSkuValue(DOMNodeList $nodes) |
||
235 | /** |
||
236 | * extract the the title value from the DOMNodelist object if value not empty |
||
237 | * simply append the store id and return the vlaue. if the value is empty |
||
238 | * append the a know string string with the sku and the store id and return |
||
239 | * @param DOMNodeList $nodes |
||
240 | * @param Mage_Catalog_Model_Product $product |
||
241 | * @return string |
||
242 | */ |
||
243 | public function extractUrlKeyValue(DOMNodeList $nodes, Mage_Catalog_Model_Product $product) |
||
250 | /** |
||
251 | * given a gift card type return the gift card constant mapped to it |
||
252 | * @param string $type the gift card type in this set (virtual, physical or combined) |
||
253 | * @see Enterprise_GiftCard_Model_Giftcard |
||
254 | * @return int|null |
||
255 | */ |
||
256 | View Code Duplication | protected function _getGiftCardType($type) |
|
267 | /** |
||
268 | * extract the giftcard tender code from the DOMNOdeList object get its map value |
||
269 | * from the config and then return the actual constant to the know magento gift card sets |
||
270 | * @param DOMNodeList $nodes |
||
271 | * @return int|null integer value a valid tender type was extracted null tender type is not configure |
||
272 | */ |
||
273 | public function extractGiftcardTenderValue(DOMNodeList $nodes) |
||
280 | |||
281 | /** |
||
282 | * given DOMNodeList object containing htscode data extract the htscode data |
||
283 | * into an array of array of keys and return a serialize string of the build array of htscode data |
||
284 | * @param DOMNodeList $nodes |
||
285 | * @return string a serialize string of array of htscodes |
||
286 | */ |
||
287 | public function extractHtsCodesValue(DOMNodeList $nodes) |
||
300 | |||
301 | /** |
||
302 | * extract the attribute set name |
||
303 | * |
||
304 | * @param DOMNodeList $nodes |
||
305 | * @param Mage_Catalog_Model_Product $product |
||
306 | * @return int |
||
307 | */ |
||
308 | public function extractAttributeSetValue(DOMNodeList $nodes, Mage_Catalog_Model_Product $product) |
||
320 | /** |
||
321 | * extract the first element of a dom node list and return a string value |
||
322 | * @param DOMNodeList $nodes |
||
323 | * @return string |
||
324 | */ |
||
325 | public function extractStringValue(DOMNodeList $nodes) |
||
329 | /** |
||
330 | * extract the first element of a dom node list and return a boolean |
||
331 | * value of the extract string |
||
332 | * @param DOMNodeList $nodes |
||
333 | * @return bool |
||
334 | */ |
||
335 | public function extractBoolValue(DOMNodeList $nodes) |
||
339 | /** |
||
340 | * extract the first element of a dom node list and return the string value cast as integer value |
||
341 | * @param DOMNodeList $nodes |
||
342 | * @return int |
||
343 | */ |
||
344 | public function extractIntValue(DOMNodeList $nodes) |
||
348 | /** |
||
349 | * extract the first element of a dom node list and return the string value cast as float value |
||
350 | * @param DOMNodeList $nodes |
||
351 | * @return int |
||
352 | */ |
||
353 | public function extractFloatValue(DOMNodeList $nodes) |
||
357 | /** |
||
358 | * it return the pass in value parameter |
||
359 | * it's a callback to return static value set in the config |
||
360 | * @param mixed $value |
||
361 | * @return mixed |
||
362 | */ |
||
363 | public function passThrough($value) |
||
367 | /** |
||
368 | * Always return false. |
||
369 | * This is useful for clearing a value to have it fallback to a higher scope. |
||
370 | */ |
||
371 | public function extractFalse() |
||
375 | |||
376 | /** |
||
377 | * return a sum of the data for all elements retrieved by the xpath. |
||
378 | * @param DOMNodeList $nodes |
||
379 | * @return float |
||
380 | */ |
||
381 | public function extractFloatSum(DOMNodeList $nodes) |
||
389 | |||
390 | /** |
||
391 | * Return a negative sum of the data for all elements retrieved by the xpath. |
||
392 | * Used to get a negative amount for discount sums. |
||
393 | * @param DOMNodeList $nodes |
||
394 | * @return float |
||
395 | */ |
||
396 | public function extractDiscountSum(DOMNodeList $nodes) |
||
400 | |||
401 | /** |
||
402 | * extract all custom attributes and set it to the passed in product object. |
||
403 | * |
||
404 | * @param DOMNodeList |
||
405 | * @param Mage_Catalog_Model_Product |
||
406 | * @return null |
||
407 | */ |
||
408 | public function extractCustomAttributes(DOMNodeList $nodes, Mage_Catalog_Model_Product $product) |
||
421 | |||
422 | /** |
||
423 | * If the "ExtendedAttributes/AllowGiftMessage" node exist then we proceed |
||
424 | * to set the product attribute "use_config_gift_message_available" to false |
||
425 | * and return the boolean value of the "ExtendedAttributes/AllowGiftMessage" node. |
||
426 | * Otherwise, if the "ExtendedAttributes/AllowGiftMessage" node doesn't exists |
||
427 | * we simply return null in order to let the default value be set on the attribute. |
||
428 | * |
||
429 | * @param DOMNodeList |
||
430 | * @param Mage_Catalog_Model_Product |
||
431 | * @return int | null |
||
432 | */ |
||
433 | public function extractAllowGiftMessage(DOMNodeList $nodes, Mage_Catalog_Model_Product $product) |
||
443 | |||
444 | /** |
||
445 | * Map an array of website ids the product should be linked to, based upon |
||
446 | * the item's client id, catalog id and/or store id. |
||
447 | * |
||
448 | * @param DOMNodeList |
||
449 | * @param Mage_Catalog_Model_Product |
||
450 | * @return array |
||
451 | */ |
||
452 | public function extractWebsiteIds(DOMNodeList $nodes, Mage_Catalog_Model_Product $product) |
||
488 | } |
||
489 |
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.