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 RestIncomingQueryParamMetadata 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 RestIncomingQueryParamMetadata, and based on these observations, apply Extract Interface, too.
| 1 | <?php |
||
| 27 | class RestIncomingQueryParamMetadata |
||
| 28 | { |
||
| 29 | private $query_param_key; |
||
| 30 | private $query_param_value; |
||
| 31 | /** |
||
| 32 | * @var RestIncomingQueryParamContext |
||
| 33 | */ |
||
| 34 | private $context; |
||
| 35 | |||
| 36 | /** |
||
| 37 | * @var EE_Model_Field_Base|null |
||
| 38 | */ |
||
| 39 | private $field; |
||
| 40 | |||
| 41 | /** |
||
| 42 | * @var string same as $query_param_key but has the * and anything after it removed |
||
| 43 | */ |
||
| 44 | private $query_param_key_sans_stars; |
||
| 45 | |||
| 46 | /** |
||
| 47 | * @var string for timezone or timezone offset |
||
| 48 | */ |
||
| 49 | private $timezone; |
||
| 50 | |||
| 51 | /** |
||
| 52 | * @var boolean if the field in $query_param_key is for a GMT field (eg `EVT_modified_gmt`) |
||
| 53 | */ |
||
| 54 | private $is_gmt_field = false; |
||
| 55 | |||
| 56 | /** |
||
| 57 | * RestIncomingQueryParamMetadata constructor. |
||
| 58 | * You probably want to call |
||
| 59 | * @param string $query_param_key |
||
| 60 | * @param string $query_param_value |
||
| 61 | * @param RestIncomingQueryParamContext $context |
||
| 62 | */ |
||
| 63 | public function __construct($query_param_key, $query_param_value, RestIncomingQueryParamContext $context) |
||
| 70 | |||
| 71 | /** |
||
| 72 | * Gets the query parameter key. This may have been modified (see setQueryParamValue()) |
||
| 73 | * @return string |
||
| 74 | */ |
||
| 75 | public function getQueryParamKey() |
||
| 79 | |||
| 80 | /** |
||
| 81 | * Modifies the query parameter key passed in (Eg this is done when rewriting the simplified specified operator REST |
||
| 82 | * query parameters into the legacy structure) |
||
| 83 | * @param string|array|int|float $query_param_value |
||
| 84 | */ |
||
| 85 | private function setQueryParamValue($query_param_value) |
||
| 89 | |||
| 90 | /** |
||
| 91 | * Gets the original query parameter value passed in. |
||
| 92 | * @return string |
||
| 93 | */ |
||
| 94 | public function getQueryParamValue() |
||
| 98 | |||
| 99 | /** |
||
| 100 | * Gets the context object. |
||
| 101 | * @return RestIncomingQueryParamContext |
||
| 102 | */ |
||
| 103 | public function getContext() |
||
| 107 | |||
| 108 | /** |
||
| 109 | * Sets the query parameter key. This may be used to rewrite a key into its non-GMT alternative. |
||
| 110 | * @param string $query_param_key |
||
| 111 | */ |
||
| 112 | private function setQueryParamKey($query_param_key) |
||
| 116 | |||
| 117 | /** |
||
| 118 | * Gets the field the query parameter key indicated. This may be null (in cases where the query parameter key |
||
| 119 | * did not indicate a field, eg if it were `OR`). |
||
| 120 | * @return EE_Model_Field_Base|null |
||
| 121 | */ |
||
| 122 | public function getField() |
||
| 126 | |||
| 127 | /** |
||
| 128 | * Gets the query parameter key (with the star and everything afterwards removed). |
||
| 129 | * @return string |
||
| 130 | */ |
||
| 131 | public function getQueryParamKeySansStars() |
||
| 135 | |||
| 136 | /** |
||
| 137 | * Gets the timezone associated with this model (the site timezone, except for GMT datetime fields). |
||
| 138 | * @return string |
||
| 139 | */ |
||
| 140 | public function getTimezone() |
||
| 144 | |||
| 145 | /** |
||
| 146 | * Returns whether or not this is a GMT field |
||
| 147 | * @return boolean |
||
| 148 | */ |
||
| 149 | public function isGmtField() |
||
| 153 | |||
| 154 | /** |
||
| 155 | * Sets the field indicated by the query parameter key (might be null). |
||
| 156 | * @param EE_Model_Field_Base|null $field |
||
| 157 | */ |
||
| 158 | private function setField(EE_Model_Field_Base $field = null) |
||
| 162 | |||
| 163 | /** |
||
| 164 | * Sets the query parameter key-with-stars-removed. |
||
| 165 | * @param string $query_param_key_sans_stars |
||
| 166 | */ |
||
| 167 | private function setQueryParamKeySansStars($query_param_key_sans_stars) |
||
| 171 | |||
| 172 | /** |
||
| 173 | * Sets the timezone (this could be a timezeon offset string). |
||
| 174 | * @param string $timezone |
||
| 175 | */ |
||
| 176 | private function setTimezone($timezone) |
||
| 180 | |||
| 181 | /** |
||
| 182 | * @param mixed $is_gmt_field |
||
| 183 | */ |
||
| 184 | private function setIsGmtField($is_gmt_field) |
||
| 188 | |||
| 189 | /** |
||
| 190 | * Determines what field, query param name, and query param name without stars, and timezone to use. |
||
| 191 | * @since 4.9.72.p |
||
| 192 | * @type EE_Model_Field_Base $field |
||
| 193 | * @return void { |
||
| 194 | * @throws EE_Error |
||
| 195 | * @throws InvalidDataTypeException |
||
| 196 | * @throws InvalidInterfaceException |
||
| 197 | * @throws InvalidArgumentException |
||
| 198 | */ |
||
| 199 | private function determineFieldAndTimezone() |
||
| 228 | |||
| 229 | /** |
||
| 230 | * Given a ton of input, determines the value to use for the models. |
||
| 231 | * @since 4.9.72.p |
||
| 232 | * @return array|null |
||
| 233 | * @throws DomainException |
||
| 234 | * @throws EE_Error |
||
| 235 | * @throws RestException |
||
| 236 | * @throws DomainException |
||
| 237 | */ |
||
| 238 | public function determineConditionsQueryParameterValue() |
||
| 250 | |||
| 251 | /** |
||
| 252 | * Given that the array value provided was itself an array, handles finding the correct value to pass to the model. |
||
| 253 | * @since 4.9.72.p |
||
| 254 | * @return array|null |
||
| 255 | * @throws RestException |
||
| 256 | */ |
||
| 257 | private function determineModelValueGivenRestInputArray() |
||
| 296 | |||
| 297 | /** |
||
| 298 | * Returns if this request is a "read" request and the value provided was an array. |
||
| 299 | * This will indicate is such things as `array('<', 123)` and `array('IN', array(1,2,3))` are acceptable or not. |
||
| 300 | * @since 4.9.72.p |
||
| 301 | * @return boolean |
||
| 302 | */ |
||
| 303 | private function valueIsArrayDuringRead() |
||
| 307 | |||
| 308 | /** |
||
| 309 | * Returns if the value provided was an associative array (we should have already verified it's an array of some |
||
| 310 | * sort). If the value is an associative array, it had better be in the simplified specified operator structure. |
||
| 311 | * @since 4.9.72.p |
||
| 312 | * @return boolean |
||
| 313 | */ |
||
| 314 | private function valueIsAssociativeArray() |
||
| 318 | |||
| 319 | /** |
||
| 320 | * Checks if the array value is itself an array that fits into the simplified specified operator structure |
||
| 321 | * (eg `array('!=' => 123)`). |
||
| 322 | * @since 4.9.72.p |
||
| 323 | * @return boolean |
||
| 324 | */ |
||
| 325 | private function valueIsSimplifiedSpecifiedOperator() |
||
| 333 | |||
| 334 | /** |
||
| 335 | * Throws an exception if the sub-value is an array (eg `array('!=' => array())`). It needs to just be a string, |
||
| 336 | * of either comma-separated-values, or a JSON array. |
||
| 337 | * @since 4.9.72.p |
||
| 338 | * @param $sub_array_key |
||
| 339 | * @param $sub_array_value |
||
| 340 | * @throws RestException |
||
| 341 | */ |
||
| 342 | private function assertSubValueIsntArray($sub_array_key, $sub_array_value) |
||
| 361 | |||
| 362 | /** |
||
| 363 | * Determines if the sub-array key is an operator taking 3 or more operators. |
||
| 364 | * @since 4.9.72.p |
||
| 365 | * @param $sub_array_key |
||
| 366 | * @return boolean |
||
| 367 | */ |
||
| 368 | private function subArrayKeyIsNonBinaryOperator($sub_array_key) |
||
| 378 | |||
| 379 | /** |
||
| 380 | * Given that the $sub_array_key is a string, checks if it's an operator taking only 1 argument. |
||
| 381 | * @since 4.9.72.p |
||
| 382 | * @param string $sub_array_key |
||
| 383 | * @return boolean |
||
| 384 | */ |
||
| 385 | private function subArrayKeyIsUnaryOperator($sub_array_key) |
||
| 392 | |||
| 393 | /** |
||
| 394 | * Parses the $sub_array_value string into an array (given it could either be a comma-separated-list or a JSON |
||
| 395 | * array). eg `"1,2,3"` or `"[1,2,3]"` into `array(1,2,3)`. |
||
| 396 | * @since 4.9.72.p |
||
| 397 | * @param $sub_array_value |
||
| 398 | * @return array|mixed|object |
||
| 399 | */ |
||
| 400 | private function extractQuickStyleSpecifiedOperatorValue($sub_array_value) |
||
| 417 | |||
| 418 | /** |
||
| 419 | * Throws an exception if the value isn't a simplified specified operator (only called when we expect that). |
||
| 420 | * @since 4.9.72.p |
||
| 421 | * @throws RestException |
||
| 422 | */ |
||
| 423 | View Code Duplication | private function assertSimplifiedSpecifiedOperator() |
|
| 442 | |||
| 443 | /** |
||
| 444 | * If query_param_value were in the simplified specific operator structure, change it into the legacy structure. |
||
| 445 | * @since 4.9.72.p |
||
| 446 | * @throws RestException |
||
| 447 | */ |
||
| 448 | private function transformSimplifiedSpecifiedOperatorSyntaxIntoStandardSyntax() |
||
| 469 | |||
| 470 | /** |
||
| 471 | * Returns true is the value is an array using the legacy structure to specify the operator. Eg `array('!=',123)`. |
||
| 472 | * @since 4.9.72.p |
||
| 473 | * @return boolean |
||
| 474 | */ |
||
| 475 | private function valueIsLegacySpecifiedOperator() |
||
| 482 | |||
| 483 | /** |
||
| 484 | * Returns true if the value specified operator accepts arbitrary number of arguments, like "IN". |
||
| 485 | * @since 4.9.72.p |
||
| 486 | * @param $operator |
||
| 487 | * @return boolean |
||
| 488 | */ |
||
| 489 | View Code Duplication | private function operatorIsNAry($operator) |
|
| 500 | |||
| 501 | /** |
||
| 502 | * Returns true if the operator accepts 3 arguments (eg "BETWEEN"). |
||
| 503 | * So we're looking for a value that looks like |
||
| 504 | * `array('BETWEEN', array('2015-01-01T00:00:00', '2016-01-01T00:00:00'))`. |
||
| 505 | * @since 4.9.72.p |
||
| 506 | * @param $operator |
||
| 507 | * @return boolean |
||
| 508 | */ |
||
| 509 | private function operatorIsTernary($operator) |
||
| 519 | |||
| 520 | /** |
||
| 521 | * Returns true if the operator is a similar to LIKE, indicating the value may have wildcards we should leave alone. |
||
| 522 | * @since 4.9.72.p |
||
| 523 | * @param $operator |
||
| 524 | * @return boolean |
||
| 525 | */ |
||
| 526 | View Code Duplication | private function operatorIsLike($operator) |
|
| 533 | |||
| 534 | /** |
||
| 535 | * Returns true if the operator only takes one argument (eg it's like `IS NULL`). |
||
| 536 | * @since 4.9.72.p |
||
| 537 | * @param $operator |
||
| 538 | * @return boolean |
||
| 539 | */ |
||
| 540 | private function operatorIsUnary($operator) |
||
| 546 | |||
| 547 | /** |
||
| 548 | * Returns true if the operator specified is a binary opeator (eg `=`, `!=`) |
||
| 549 | * @since 4.9.72.p |
||
| 550 | * @param $operator |
||
| 551 | * @return boolean |
||
| 552 | */ |
||
| 553 | private function operatorisBinary($operator) |
||
| 569 | |||
| 570 | /** |
||
| 571 | * If we're debugging, throws an exception saying that the wrong number of arguments was provided. |
||
| 572 | * @since 4.9.72.p |
||
| 573 | * @param $operator |
||
| 574 | * @throws RestException |
||
| 575 | */ |
||
| 576 | private function throwWrongNumberOfArgsExceptionIfDebugging($operator) |
||
| 594 | |||
| 595 | /** |
||
| 596 | * Wrapper for ModelDataTranslator::prepareFieldValuesFromJson(), just a tad more DRY. |
||
| 597 | * @since 4.9.72.p |
||
| 598 | * @param $value |
||
| 599 | * @return mixed |
||
| 600 | * @throws RestException |
||
| 601 | */ |
||
| 602 | private function prepareValuesFromJson($value) |
||
| 611 | |||
| 612 | /** |
||
| 613 | * Throws an exception if an invalid operator was specified and we're debugging. |
||
| 614 | * @since 4.9.72.p |
||
| 615 | * @throws RestException |
||
| 616 | */ |
||
| 617 | View Code Duplication | private function throwInvalidOperatorExceptionIfDebugging() |
|
| 637 | |||
| 638 | /** |
||
| 639 | * Returns true if the query_param_key was a logic query parameter, eg `OR`, `AND`, `NOT`, `OR*`, etc. |
||
| 640 | * @since 4.9.72.p |
||
| 641 | * @return boolean |
||
| 642 | */ |
||
| 643 | private function isLogicQueryParam() |
||
| 647 | |||
| 648 | |||
| 649 | /** |
||
| 650 | * If the query param isn't for a field, it must be a nested query parameter which requires different logic. |
||
| 651 | * @since 4.9.72.p |
||
| 652 | * @return array |
||
| 653 | * @throws DomainException |
||
| 654 | * @throws EE_Error |
||
| 655 | * @throws RestException |
||
| 656 | * @throws InvalidDataTypeException |
||
| 657 | * @throws InvalidInterfaceException |
||
| 658 | * @throws InvalidArgumentException |
||
| 659 | */ |
||
| 660 | public function determineNestedConditionQueryParameters() |
||
| 704 | } |
||
| 705 | // End of file RestQueryParamMetadata.php |
||
| 707 |
Unless you are absolutely sure that the expression can never be null because of other conditions, we strongly recommend to add an additional type check to your code: