bb-drummer /
my-application
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 Languagemenu 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 Languagemenu, and based on these observations, apply Extract Interface, too.
| 1 | <?php |
||
| 21 | class Languagemenu extends \UIComponents\View\Helper\AbstractHelper |
||
| 22 | |||
| 23 | { |
||
| 24 | |||
| 25 | /** |
||
| 26 | * @var Detector $detector |
||
| 27 | */ |
||
| 28 | protected $detector; |
||
| 29 | |||
| 30 | /** |
||
| 31 | * Set the class to be used on the list container |
||
| 32 | * |
||
| 33 | * @var string || null |
||
| 34 | */ |
||
| 35 | protected $class; |
||
| 36 | |||
| 37 | /** |
||
| 38 | * Method used to construct a title for each item |
||
| 39 | * |
||
| 40 | * @var string || null |
||
| 41 | */ |
||
| 42 | protected $titleMethod = 'displayLanguage'; |
||
| 43 | |||
| 44 | /** |
||
| 45 | * Flag to specify specifies whether the title should be in the current locale |
||
| 46 | * |
||
| 47 | * @var boolean default false |
||
| 48 | */ |
||
| 49 | protected $titleInCurrentLocale = false; |
||
| 50 | |||
| 51 | /** |
||
| 52 | * Method used to construct a label for each item |
||
| 53 | * |
||
| 54 | * @var string || null |
||
| 55 | */ |
||
| 56 | protected $labelMethod = 'displayLanguage'; |
||
| 57 | |||
| 58 | /** |
||
| 59 | * Flag to specify specifies whether the label should be in the current locale |
||
| 60 | * |
||
| 61 | * @var boolean default true |
||
| 62 | */ |
||
| 63 | protected $labelInCurrentLocale = true; |
||
| 64 | |||
| 65 | /** |
||
| 66 | * Flag to specify the current locale should be omitted from the menu |
||
| 67 | * |
||
| 68 | * @var boolean default false |
||
| 69 | */ |
||
| 70 | protected $omitCurrent = false; |
||
| 71 | |||
| 72 | /** |
||
| 73 | * default CSS class to use for li elements |
||
| 74 | * |
||
| 75 | * @var string |
||
| 76 | */ |
||
| 77 | protected $defaultLiClass = ''; |
||
| 78 | |||
| 79 | /** |
||
| 80 | * CSS class to use for the ul sub-menu element |
||
| 81 | * |
||
| 82 | * @var string |
||
| 83 | */ |
||
| 84 | protected $subUlClass = 'dropdown-menu'; |
||
| 85 | |||
| 86 | /** |
||
| 87 | * CSS class to use for the 1. level (NOT root level!) ul sub-menu element |
||
| 88 | * |
||
| 89 | * @var string |
||
| 90 | */ |
||
| 91 | protected $subUlClassLevel1 = 'dropdown-menu'; |
||
| 92 | |||
| 93 | /** |
||
| 94 | * CSS class to use for the active li sub-menu element |
||
| 95 | * |
||
| 96 | * @var string |
||
| 97 | */ |
||
| 98 | protected $subLiClass = 'dropdown-submenu'; |
||
| 99 | |||
| 100 | /** |
||
| 101 | * CSS class to use for the active li sub-menu element |
||
| 102 | * |
||
| 103 | * @var string |
||
| 104 | */ |
||
| 105 | protected $subLiClassLevel0 = 'dropdown'; |
||
| 106 | |||
| 107 | /** |
||
| 108 | * CSS class prefix to use for the menu element's icon class |
||
| 109 | * |
||
| 110 | * @var string |
||
| 111 | */ |
||
| 112 | protected $iconPrefixClass = 'icon-'; |
||
| 113 | |||
| 114 | /** |
||
| 115 | * HREF string to use for the sub-menu toggle element's HREF attribute, |
||
| 116 | * to override current page's href/'htmlify' setting |
||
| 117 | * |
||
| 118 | * @var string |
||
| 119 | */ |
||
| 120 | protected $hrefSubToggleOverride = null; |
||
| 121 | |||
| 122 | /** |
||
| 123 | * Partial view script to use for rendering menu link/item |
||
| 124 | * |
||
| 125 | * @var string|array |
||
| 126 | */ |
||
| 127 | protected $htmlifyPartial = null; |
||
| 128 | |||
| 129 | /** |
||
| 130 | * @param Detector $detector |
||
| 131 | */ |
||
| 132 | public function setDetector($detector) |
||
| 137 | |||
| 138 | /** |
||
| 139 | * @return Detector $detector |
||
| 140 | */ |
||
| 141 | public function getDetector() |
||
| 152 | |||
| 153 | /** |
||
| 154 | * @param string $class |
||
| 155 | */ |
||
| 156 | public function setUlClass($class) |
||
| 161 | |||
| 162 | /** |
||
| 163 | * @return string |
||
| 164 | */ |
||
| 165 | public function getUlClass() |
||
| 169 | |||
| 170 | /** |
||
| 171 | * @param string $itemTitleMethod |
||
| 172 | */ |
||
| 173 | public function setTitleMethod($titleMethod) |
||
| 180 | |||
| 181 | /** |
||
| 182 | * @return string |
||
| 183 | */ |
||
| 184 | public function getTitleMethod() |
||
| 188 | |||
| 189 | /** |
||
| 190 | * @param boolean $flag |
||
| 191 | */ |
||
| 192 | public function setTitleInCurrentLocale($flag) |
||
| 197 | |||
| 198 | /** |
||
| 199 | * @return boolean |
||
| 200 | */ |
||
| 201 | public function getTitleInCurrentLocale() |
||
| 205 | |||
| 206 | /** |
||
| 207 | * @param string $labelMethod |
||
| 208 | */ |
||
| 209 | public function setLabelMethod($labelMethod) |
||
| 216 | |||
| 217 | /** |
||
| 218 | * @return string |
||
| 219 | */ |
||
| 220 | public function getLabelMethod() |
||
| 224 | |||
| 225 | /** |
||
| 226 | * @param boolean $flag |
||
| 227 | */ |
||
| 228 | public function setLabelInCurrentLocale($flag) |
||
| 233 | |||
| 234 | /** |
||
| 235 | * @return boolean |
||
| 236 | */ |
||
| 237 | public function getLabelInCurrentLocale() |
||
| 241 | |||
| 242 | /** |
||
| 243 | * @param boolean $omitCurrent |
||
| 244 | */ |
||
| 245 | public function setOmitCurrent($omitCurrent) |
||
| 250 | |||
| 251 | /** |
||
| 252 | * @return boolean |
||
| 253 | */ |
||
| 254 | public function omitCurrent() |
||
| 258 | |||
| 259 | /** |
||
| 260 | * @return the $defaultLiClass |
||
| 261 | */ |
||
| 262 | public function getDefaultLiClass() { |
||
| 265 | |||
| 266 | /** |
||
| 267 | * @param string $defaultLiClass |
||
| 268 | */ |
||
| 269 | public function setDefaultLiClass($defaultLiClass) { |
||
| 273 | |||
| 274 | /** |
||
| 275 | * @return the $subUlClass |
||
| 276 | */ |
||
| 277 | public function getSubUlClass() { |
||
| 280 | |||
| 281 | /** |
||
| 282 | * @param string $subUlClass |
||
| 283 | */ |
||
| 284 | public function setSubUlClass($subUlClass) { |
||
| 288 | |||
| 289 | /** |
||
| 290 | * @return the $subUlClassLevel1 |
||
| 291 | */ |
||
| 292 | public function getSubUlClassLevel1() { |
||
| 295 | |||
| 296 | /** |
||
| 297 | * @param string $subUlClassLevel1 |
||
| 298 | */ |
||
| 299 | public function setSubUlClassLevel1($subUlClassLevel1) { |
||
| 303 | |||
| 304 | /** |
||
| 305 | * @return the $subLiClass |
||
| 306 | */ |
||
| 307 | public function getSubLiClass() { |
||
| 310 | |||
| 311 | /** |
||
| 312 | * @param string $subLiClass |
||
| 313 | */ |
||
| 314 | public function setSubLiClass($subLiClass) { |
||
| 318 | |||
| 319 | /** |
||
| 320 | * @return the $subLiClassLevel0 |
||
| 321 | */ |
||
| 322 | public function getSubLiClassLevel0() { |
||
| 325 | |||
| 326 | /** |
||
| 327 | * @param string $subLiClassLevel0 |
||
| 328 | */ |
||
| 329 | public function setSubLiClassLevel0($subLiClassLevel0) { |
||
| 333 | |||
| 334 | /** |
||
| 335 | * @return the $iconPrefixClass |
||
| 336 | */ |
||
| 337 | public function getIconPrefixClass() { |
||
| 340 | |||
| 341 | /** |
||
| 342 | * @param string $iconPrefixClass |
||
| 343 | */ |
||
| 344 | public function setIconPrefixClass($iconPrefixClass) { |
||
| 348 | |||
| 349 | /** |
||
| 350 | * @return the $hrefSubToggleOverride |
||
| 351 | */ |
||
| 352 | public function getHrefSubToggleOverride() { |
||
| 355 | |||
| 356 | /** |
||
| 357 | * @param string $hrefSubToggleOverride |
||
| 358 | */ |
||
| 359 | public function setHrefSubToggleOverride($hrefSubToggleOverride) { |
||
| 363 | |||
| 364 | /** |
||
| 365 | * Sets which partial view script to use for rendering menu |
||
| 366 | * |
||
| 367 | * @param string|array $partial partial view script or null. If an array is |
||
| 368 | * given, it is expected to contain two |
||
| 369 | * values; the partial view script to use, |
||
| 370 | * and the module where the script can be |
||
| 371 | * found. |
||
| 372 | * @return self |
||
| 373 | */ |
||
| 374 | View Code Duplication | public function setHtmlifyPartial($partial) |
|
| 382 | |||
| 383 | /** |
||
| 384 | * Returns partial view script to use for rendering menu |
||
| 385 | * |
||
| 386 | * @return string|array|null |
||
| 387 | */ |
||
| 388 | public function getHtmlifyPartial() |
||
| 392 | |||
| 393 | /** |
||
| 394 | * View helper entry point: |
||
| 395 | * Retrieves helper and optionally sets component options to operate on |
||
| 396 | * |
||
| 397 | * @param array|StdClass $options [optional] component options to operate on |
||
| 398 | * @return self |
||
| 399 | */ |
||
| 400 | public function __invoke($options = array()) |
||
| 405 | |||
| 406 | /** |
||
| 407 | * render component |
||
| 408 | * |
||
| 409 | * @param boolean $output |
||
| 410 | * |
||
| 411 | * @return string |
||
| 412 | */ |
||
| 413 | View Code Duplication | public function render($output = false) |
|
| 430 | |||
| 431 | /** |
||
| 432 | * build markup |
||
| 433 | * |
||
| 434 | * @return string |
||
| 435 | */ |
||
| 436 | public function buildComponent() |
||
| 495 | |||
| 496 | /** |
||
| 497 | * Check whether method part of the Locale class is |
||
| 498 | * |
||
| 499 | * @param string $method Method to check |
||
| 500 | * @throws RuntimeException If method is not part of locale |
||
| 501 | * @return true |
||
| 502 | */ |
||
| 503 | protected function checkLocaleMethod($method) |
||
| 524 | |||
| 525 | /** |
||
| 526 | * Retrieves a value by property from Locale |
||
| 527 | * |
||
| 528 | * @param $property |
||
| 529 | * @param $locale |
||
| 530 | * @param bool $in_locale |
||
| 531 | * @return mixed |
||
| 532 | */ |
||
| 533 | protected function getLocaleProperty($property, $locale, $in_locale = false) |
||
| 545 | |||
| 546 | } |
Loading history...
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
$accountIdthat can either hold an Id object or false (if there is no account id yet). Your code now assigns that value to theidproperty of an instance of theAccountclass. 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.