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 LangReplacer 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 LangReplacer, and based on these observations, apply Extract Interface, too.
| 1 | <?php |
||
| 8 | class LangReplacer |
||
| 9 | { |
||
| 10 | |||
| 11 | public static $domainTranslations = []; |
||
| 12 | |||
| 13 | const LANG_FUNC = '(t?langf?)'; |
||
| 14 | |||
| 15 | const WORD = '([\s\S]+?)'; |
||
| 16 | |||
| 17 | const QUOTE = '[\'\"]{1}'; |
||
| 18 | |||
| 19 | const QUOTED_WORD = self::QUOTE . self::WORD . self::QUOTE; |
||
| 20 | |||
| 21 | const QUOTED_DOMAIN = '(?:\s*,\s*' . self::QUOTE . self::WORD . self::QUOTE . ')?'; |
||
| 22 | |||
| 23 | const QUOTED_PARAMS = '(?:\s*,\s*' . self::WORD . ')?'; |
||
| 24 | |||
| 25 | const EXPRESSION = '/' . self::LANG_FUNC . '\(' . self::QUOTED_WORD . self::QUOTED_DOMAIN . '\)/'; |
||
| 26 | |||
| 27 | |||
| 28 | /* |
||
| 29 | ******************************************************************************************************************* |
||
| 30 | * INTERFACE |
||
| 31 | * |
||
| 32 | * USAGE |
||
| 33 | * $module = 'aggregator'; |
||
| 34 | * $allModules |
||
| 35 | * LangReplacer::replaceModuleLangStrings($module); |
||
| 36 | * LangReplacer::replaceMainLangStrings(); |
||
| 37 | * LangReplacer::findCyrillicKeys($module) |
||
| 38 | ******************************************************************************************************************* |
||
| 39 | */ |
||
| 40 | |||
| 41 | public static function findCyrillicKeys($module) { |
||
| 60 | |||
| 61 | public static function replaceMainLangStrings() { |
||
| 72 | |||
| 73 | /** |
||
| 74 | * Replace key to english translation |
||
| 75 | * Add translation to other languages for this key |
||
| 76 | * @param $module |
||
| 77 | */ |
||
| 78 | public static function replaceModuleLangStrings($module) { |
||
| 88 | |||
| 89 | private static function savePoMo($saveDomain) { |
||
| 104 | |||
| 105 | /** |
||
| 106 | * |
||
| 107 | * @param array $translated |
||
| 108 | * @param string $module |
||
| 109 | */ |
||
| 110 | private static function updateKeysInMoPoFiles($translated, $module) { |
||
| 139 | |||
| 140 | /** |
||
| 141 | * @param string $file |
||
| 142 | * @param array $data |
||
| 143 | * @return array |
||
| 144 | */ |
||
| 145 | private static function changeKeysInFile($file, $data) { |
||
| 184 | |||
| 185 | public static function getMainPaths() { |
||
| 204 | |||
| 205 | /* |
||
| 206 | ******************************************************************************************************************* |
||
| 207 | * PARSE |
||
| 208 | ******************************************************************************************************************* |
||
| 209 | */ |
||
| 210 | |||
| 211 | /** |
||
| 212 | * Find lang calls in module files |
||
| 213 | * |
||
| 214 | * @param $module |
||
| 215 | * @return array |
||
| 216 | */ |
||
| 217 | public static function parseModule($module) { |
||
| 220 | |||
| 221 | /** |
||
| 222 | * Find all main lang calls in main directories |
||
| 223 | * @return array |
||
| 224 | */ |
||
| 225 | public static function parseMain() { |
||
| 237 | |||
| 238 | /** |
||
| 239 | * Search lang function call in all php|tpl files recursively |
||
| 240 | * @param $dir |
||
| 241 | * @return array |
||
| 242 | */ |
||
| 243 | public static function parseDir($dir) { |
||
| 265 | |||
| 266 | /* |
||
| 267 | ******************************************************************************************************************* |
||
| 268 | * Translate |
||
| 269 | ******************************************************************************************************************* |
||
| 270 | */ |
||
| 271 | |||
| 272 | /** |
||
| 273 | * @param $domain |
||
| 274 | * @param $word |
||
| 275 | * @param string $lang |
||
| 276 | * @return null|string |
||
| 277 | */ |
||
| 278 | public static function translate($domain, $word, $lang = 'en_US') { |
||
| 287 | |||
| 288 | /** |
||
| 289 | * @param $domain |
||
| 290 | * @param string $locale |
||
| 291 | * @return Translations |
||
| 292 | */ |
||
| 293 | public static function getDomainTranslations($domain, $locale = 'en_US') { |
||
| 301 | |||
| 302 | /* |
||
| 303 | ******************************************************************************************************************* |
||
| 304 | * REGEXP |
||
| 305 | ******************************************************************************************************************* |
||
| 306 | */ |
||
| 307 | |||
| 308 | /** |
||
| 309 | * Find all language calls in string |
||
| 310 | * @param $fileContent |
||
| 311 | * @param string $word |
||
| 312 | * @return array |
||
| 313 | */ |
||
| 314 | public static function find($fileContent, $word = '') { |
||
| 319 | |||
| 320 | public static function isCyrillic($word) { |
||
| 323 | |||
| 324 | /** |
||
| 325 | * Create lang function call |
||
| 326 | * @param $function |
||
| 327 | * @param $word |
||
| 328 | * @param $domain |
||
| 329 | * @param $params |
||
| 330 | * @return string |
||
| 331 | */ |
||
| 332 | public static function createReplacement($function, $word, $domain, $params) { |
||
| 335 | |||
| 336 | /** |
||
| 337 | * Create regular expression for lang call |
||
| 338 | * @param $word |
||
| 339 | * @return string |
||
| 340 | */ |
||
| 341 | private static function createExpression($word = null) { |
||
| 350 | |||
| 351 | /* |
||
| 352 | ******************************************************************************************************************* |
||
| 353 | * PATHS |
||
| 354 | ******************************************************************************************************************* |
||
| 355 | */ |
||
| 356 | |||
| 357 | /** |
||
| 358 | * Get lang directories for module |
||
| 359 | * |
||
| 360 | * @param $module |
||
| 361 | * @return array ['en_US', ...] |
||
| 362 | */ |
||
| 363 | public static function getModuleLanguages($module) { |
||
| 377 | |||
| 378 | /** |
||
| 379 | * Get po file path |
||
| 380 | * @param string $domain |
||
| 381 | * @param string $locale |
||
| 382 | * @return string |
||
| 383 | */ |
||
| 384 | View Code Duplication | public static function getDomainPoFilePath($domain, $locale = 'en_US') { |
|
| 393 | |||
| 394 | /** |
||
| 395 | * Create new mo file path |
||
| 396 | * @param $domain |
||
| 397 | * @param string $locale |
||
| 398 | * @return string |
||
| 399 | */ |
||
| 400 | View Code Duplication | public static function createDomainMoFilePath($domain, $locale = 'en_US') { |
|
| 408 | |||
| 409 | /** |
||
| 410 | * Languages directory |
||
| 411 | * |
||
| 412 | * @param $domain |
||
| 413 | * @param $locale |
||
| 414 | * @return string |
||
| 415 | */ |
||
| 416 | public static function getTranslationDir($domain, $locale) { |
||
| 424 | |||
| 425 | /** |
||
| 426 | * Full path to module |
||
| 427 | * @param $moduleName |
||
| 428 | * @return string |
||
| 429 | */ |
||
| 430 | public static function getModulePath($moduleName) { |
||
| 433 | |||
| 434 | public static function getAllModules() { |
||
| 445 | |||
| 446 | } |
Adding an explicit array definition is generally preferable to implicit array definition as it guarantees a stable state of the code.
Let’s take a look at an example:
As you can see in this example, the array
$myArrayis initialized the first time when the foreach loop is entered. You can also see that the value of thebarkey is only written conditionally; thus, its value might result from a previous iteration.This might or might not be intended. To make your intention clear, your code more readible and to avoid accidental bugs, we recommend to add an explicit initialization $myArray = array() either outside or inside the foreach loop.