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 Json5Decoder 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 Json5Decoder, and based on these observations, apply Extract Interface, too.
| 1 | <?php |
||
| 17 | final class Json5Decoder |
||
| 18 | { |
||
| 19 | private $json; |
||
| 20 | |||
| 21 | private $length; |
||
| 22 | |||
| 23 | private $at = 0; |
||
| 24 | |||
| 25 | private $currentByte; |
||
| 26 | |||
| 27 | private $lineNumber = 1; |
||
| 28 | |||
| 29 | private $associative = false; |
||
| 30 | |||
| 31 | private $maxDepth = 512; |
||
| 32 | |||
| 33 | private $castBigIntToString = false; |
||
| 34 | |||
| 35 | private $depth = 1; |
||
| 36 | |||
| 37 | private $currentLineStartsAt = 0; |
||
| 38 | |||
| 39 | /** |
||
| 40 | * Private constructor. |
||
| 41 | * |
||
| 42 | * @param string $json |
||
| 43 | * @param bool $associative |
||
| 44 | * @param int $depth |
||
| 45 | * @param bool $castBigIntToString |
||
| 46 | */ |
||
| 47 | 420 | private function __construct($json, $associative = false, $depth = 512, $castBigIntToString = false) |
|
| 57 | |||
| 58 | /** |
||
| 59 | * Takes a JSON encoded string and converts it into a PHP variable. |
||
| 60 | * |
||
| 61 | * The parameters exactly match PHP's json_decode() function - see |
||
| 62 | * http://php.net/manual/en/function.json-decode.php for more information. |
||
| 63 | * |
||
| 64 | * @param string $source The JSON string being decoded. |
||
| 65 | * @param bool $associative When TRUE, returned objects will be converted into associative arrays. |
||
| 66 | * @param int $depth User specified recursion depth. |
||
| 67 | * @param int $options Bitmask of JSON decode options. |
||
| 68 | * |
||
| 69 | * @return mixed |
||
| 70 | */ |
||
| 71 | 420 | public static function decode($source, $associative = false, $depth = 512, $options = 0) |
|
| 72 | { |
||
| 73 | // Try parsing with json_decode first, since that's much faster |
||
| 74 | // We only attempt this on PHP 7+ because 5.x doesn't parse some edge cases correctly |
||
| 75 | 420 | if (PHP_VERSION_ID >= 70000) { |
|
| 76 | $result = json_decode($source, $associative, $depth, $options); |
||
| 77 | if (json_last_error() === JSON_ERROR_NONE) { |
||
| 78 | return $result; |
||
| 79 | } |
||
| 80 | } |
||
| 81 | |||
| 82 | // Fall back to JSON5 if that fails |
||
| 83 | 420 | $associative = $associative === true || ($associative === null && $options & JSON_OBJECT_AS_ARRAY); |
|
| 84 | 420 | $castBigIntToString = $options & JSON_BIGINT_AS_STRING; |
|
| 85 | |||
| 86 | 420 | $decoder = new self((string)$source, $associative, $depth, $castBigIntToString); |
|
|
|
|||
| 87 | |||
| 88 | 420 | $result = $decoder->value(); |
|
| 89 | 312 | $decoder->white(); |
|
| 90 | 309 | if ($decoder->currentByte) { |
|
| 91 | 18 | $decoder->throwSyntaxError('Syntax error'); |
|
| 92 | } |
||
| 93 | |||
| 94 | 291 | return $result; |
|
| 95 | } |
||
| 96 | |||
| 97 | /** |
||
| 98 | * @param int $at |
||
| 99 | * |
||
| 100 | * @return null |
||
| 101 | */ |
||
| 102 | 420 | private function getByte($at) |
|
| 110 | |||
| 111 | /** |
||
| 112 | * @return string|null |
||
| 113 | */ |
||
| 114 | 33 | private function currentChar() |
|
| 122 | |||
| 123 | /** |
||
| 124 | * Parse the next character. |
||
| 125 | * |
||
| 126 | * @return null|string |
||
| 127 | */ |
||
| 128 | 390 | private function next() |
|
| 141 | |||
| 142 | /** |
||
| 143 | * Parse the next character if it matches $c or fail. |
||
| 144 | * |
||
| 145 | * @param string $c |
||
| 146 | * |
||
| 147 | * @return string|null |
||
| 148 | */ |
||
| 149 | 186 | private function nextOrFail($c) |
|
| 161 | |||
| 162 | /** |
||
| 163 | * Get the next character without consuming it or |
||
| 164 | * assigning it to the ch variable. |
||
| 165 | * |
||
| 166 | * @return mixed |
||
| 167 | */ |
||
| 168 | 39 | private function peek() |
|
| 172 | |||
| 173 | /** |
||
| 174 | * Attempt to match a regular expression at the current position on the current line. |
||
| 175 | * |
||
| 176 | * This function will not match across multiple lines. |
||
| 177 | * |
||
| 178 | * @param string $regex |
||
| 179 | * |
||
| 180 | * @return string|null |
||
| 181 | */ |
||
| 182 | 216 | private function match($regex) |
|
| 199 | |||
| 200 | /** |
||
| 201 | * Parse an identifier. |
||
| 202 | * |
||
| 203 | * Normally, reserved words are disallowed here, but we |
||
| 204 | * only use this for unquoted object keys, where reserved words are allowed, |
||
| 205 | * so we don't check for those here. References: |
||
| 206 | * - http://es5.github.com/#x7.6 |
||
| 207 | * - https://developer.mozilla.org/en/Core_JavaScript_1.5_Guide/Core_Language_Features#Variables |
||
| 208 | * - http://docstore.mik.ua/orelly/webprog/jscript/ch02_07.htm |
||
| 209 | */ |
||
| 210 | 42 | private function identifier() |
|
| 228 | |||
| 229 | 222 | private function number() |
|
| 304 | |||
| 305 | 93 | private function string() |
|
| 348 | |||
| 349 | /** |
||
| 350 | * Skip an inline comment, assuming this is one. |
||
| 351 | * |
||
| 352 | * The current character should be the second / character in the // pair that begins this inline comment. |
||
| 353 | * To finish the inline comment, we look for a newline or the end of the text. |
||
| 354 | */ |
||
| 355 | 36 | private function inlineComment() |
|
| 366 | |||
| 367 | /** |
||
| 368 | * Skip a block comment, assuming this is one. |
||
| 369 | * |
||
| 370 | * The current character should be the * character in the /* pair that begins this block comment. |
||
| 371 | * To finish the block comment, we look for an ending */ pair of characters, |
||
| 372 | * but we also watch for the end of text before the comment is terminated. |
||
| 373 | */ |
||
| 374 | 21 | private function blockComment() |
|
| 390 | |||
| 391 | /** |
||
| 392 | * Skip a comment, whether inline or block-level, assuming this is one. |
||
| 393 | */ |
||
| 394 | 57 | private function comment() |
|
| 407 | |||
| 408 | /** |
||
| 409 | * Skip whitespace and comments. |
||
| 410 | * |
||
| 411 | * Note that we're detecting comments by only a single / character. |
||
| 412 | * This works since regular expressions are not valid JSON(5), but this will |
||
| 413 | * break if there are other valid values that begin with a / character! |
||
| 414 | */ |
||
| 415 | 420 | private function white() |
|
| 431 | |||
| 432 | /** |
||
| 433 | * Matches true, false, null, etc |
||
| 434 | */ |
||
| 435 | 93 | private function word() |
|
| 476 | |||
| 477 | 45 | private function arr() |
|
| 515 | |||
| 516 | /** |
||
| 517 | * Parse an object value |
||
| 518 | */ |
||
| 519 | 96 | private function obj() |
|
| 565 | |||
| 566 | /** |
||
| 567 | * Parse a JSON value. |
||
| 568 | * |
||
| 569 | * It could be an object, an array, a string, a number, |
||
| 570 | * or a word. |
||
| 571 | */ |
||
| 572 | 420 | private function value() |
|
| 591 | |||
| 592 | 129 | private function throwSyntaxError($message) |
|
| 600 | |||
| 601 | 33 | private static function renderChar($chr) |
|
| 605 | |||
| 606 | /** |
||
| 607 | * @param string $ch |
||
| 608 | * |
||
| 609 | * @return string|null |
||
| 610 | */ |
||
| 611 | 18 | private static function getEscapee($ch) |
|
| 629 | } |
||
| 630 |
It seems like the type of the argument is not accepted by the function/method which you are calling.
In some cases, in particular if PHP’s automatic type-juggling kicks in this might be fine. In other cases, however this might be a bug.
We suggest to add an explicit type cast like in the following example: