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 |
||
29 | class RestIncomingQueryParamMetadata |
||
30 | { |
||
31 | private $query_param_key; |
||
32 | private $query_param_value; |
||
33 | /** |
||
34 | * @var RestIncomingQueryParamContext |
||
35 | */ |
||
36 | private $context; |
||
37 | |||
38 | /** |
||
39 | * @var EE_Model_Field_Base|null |
||
40 | */ |
||
41 | private $field; |
||
42 | |||
43 | /** |
||
44 | * @var string same as $query_param_key but has the * and anything after it removed |
||
45 | */ |
||
46 | private $query_param_key_sans_stars; |
||
47 | |||
48 | /** |
||
49 | * @var string for timezone or timezone offset |
||
50 | */ |
||
51 | private $timezone; |
||
52 | |||
53 | /** |
||
54 | * @var boolean if the field in $query_param_key is for a GMT field (eg `EVT_modified_gmt`) |
||
55 | */ |
||
56 | private $is_gmt_field = false; |
||
57 | |||
58 | /** |
||
59 | * RestIncomingQueryParamMetadata constructor. |
||
60 | * You probably want to call |
||
61 | * @param string $query_param_key |
||
62 | * @param string $query_param_value |
||
63 | * @param RestIncomingQueryParamContext $context |
||
64 | */ |
||
65 | public function __construct($query_param_key, $query_param_value, RestIncomingQueryParamContext $context) |
||
72 | |||
73 | /** |
||
74 | * Gets the query parameter key. This may have been modified (see setQueryParamValue()) |
||
75 | * @return string |
||
76 | */ |
||
77 | public function getQueryParamKey() |
||
81 | |||
82 | /** |
||
83 | * Modifies the query parameter key passed in (Eg this is done when rewriting the simplified specified operator REST |
||
84 | * query parameters into the legacy structure) |
||
85 | * @param string|array|int|float $query_param_value |
||
86 | */ |
||
87 | private function setQueryParamValue($query_param_value) |
||
91 | |||
92 | /** |
||
93 | * Gets the original query parameter value passed in. |
||
94 | * @return string |
||
95 | */ |
||
96 | public function getQueryParamValue() |
||
100 | |||
101 | /** |
||
102 | * Gets the context object. |
||
103 | * @return RestIncomingQueryParamContext |
||
104 | */ |
||
105 | public function getContext() |
||
109 | |||
110 | /** |
||
111 | * Sets the query parameter key. This may be used to rewrite a key into its non-GMT alternative. |
||
112 | * @param string $query_param_key |
||
113 | */ |
||
114 | private function setQueryParamKey($query_param_key) |
||
118 | |||
119 | /** |
||
120 | * Gets the field the query parameter key indicated. This may be null (in cases where the query parameter key |
||
121 | * did not indicate a field, eg if it were `OR`). |
||
122 | * @return EE_Model_Field_Base|null |
||
123 | */ |
||
124 | public function getField() |
||
128 | |||
129 | /** |
||
130 | * Gets the query parameter key (with the star and everything afterwards removed). |
||
131 | * @return string |
||
132 | */ |
||
133 | public function getQueryParamKeySansStars() |
||
137 | |||
138 | /** |
||
139 | * Gets the timezone associated with this model (the site timezone, except for GMT datetime fields). |
||
140 | * @return string |
||
141 | */ |
||
142 | public function getTimezone() |
||
146 | |||
147 | /** |
||
148 | * Returns whether or not this is a GMT field |
||
149 | * @return boolean |
||
150 | */ |
||
151 | public function isGmtField() |
||
155 | |||
156 | /** |
||
157 | * Sets the field indicated by the query parameter key (might be null). |
||
158 | * @param EE_Model_Field_Base|null $field |
||
159 | */ |
||
160 | private function setField(EE_Model_Field_Base $field = null) |
||
164 | |||
165 | /** |
||
166 | * Sets the query parameter key-with-stars-removed. |
||
167 | * @param string $query_param_key_sans_stars |
||
168 | */ |
||
169 | private function setQueryParamKeySansStars($query_param_key_sans_stars) |
||
173 | |||
174 | /** |
||
175 | * Sets the timezone (this could be a timezeon offset string). |
||
176 | * @param string $timezone |
||
177 | */ |
||
178 | private function setTimezone($timezone) |
||
182 | |||
183 | /** |
||
184 | * @param mixed $is_gmt_field |
||
185 | */ |
||
186 | private function setIsGmtField($is_gmt_field) |
||
190 | |||
191 | /** |
||
192 | * Determines what field, query param name, and query param name without stars, and timezone to use. |
||
193 | * @since 4.9.72.p |
||
194 | * @type EE_Model_Field_Base $field |
||
195 | * @return void { |
||
196 | * @throws EE_Error |
||
197 | * @throws InvalidDataTypeException |
||
198 | * @throws InvalidInterfaceException |
||
199 | * @throws InvalidArgumentException |
||
200 | */ |
||
201 | private function determineFieldAndTimezone() |
||
231 | |||
232 | /** |
||
233 | * Throws an exception if a non-admin is trying to query by password. |
||
234 | * @since 4.9.74.p |
||
235 | * @throws RestException |
||
236 | */ |
||
237 | private function assertOnlyAdminCanReadPasswordFields() |
||
253 | |||
254 | /** |
||
255 | * Given a ton of input, determines the value to use for the models. |
||
256 | * @since 4.9.72.p |
||
257 | * @return array|null |
||
258 | * @throws DomainException |
||
259 | * @throws EE_Error |
||
260 | * @throws RestException |
||
261 | * @throws DomainException |
||
262 | */ |
||
263 | public function determineConditionsQueryParameterValue() |
||
275 | |||
276 | /** |
||
277 | * Given that the array value provided was itself an array, handles finding the correct value to pass to the model. |
||
278 | * @since 4.9.72.p |
||
279 | * @return array|null |
||
280 | * @throws RestException |
||
281 | */ |
||
282 | private function determineModelValueGivenRestInputArray() |
||
321 | |||
322 | /** |
||
323 | * Returns if this request is a "read" request and the value provided was an array. |
||
324 | * This will indicate is such things as `array('<', 123)` and `array('IN', array(1,2,3))` are acceptable or not. |
||
325 | * @since 4.9.72.p |
||
326 | * @return boolean |
||
327 | */ |
||
328 | private function valueIsArrayDuringRead() |
||
332 | |||
333 | /** |
||
334 | * Returns if the value provided was an associative array (we should have already verified it's an array of some |
||
335 | * sort). If the value is an associative array, it had better be in the simplified specified operator structure. |
||
336 | * @since 4.9.72.p |
||
337 | * @return boolean |
||
338 | */ |
||
339 | private function valueIsAssociativeArray() |
||
343 | |||
344 | /** |
||
345 | * Checks if the array value is itself an array that fits into the simplified specified operator structure |
||
346 | * (eg `array('!=' => 123)`). |
||
347 | * @since 4.9.72.p |
||
348 | * @return boolean |
||
349 | */ |
||
350 | private function valueIsSimplifiedSpecifiedOperator() |
||
358 | |||
359 | /** |
||
360 | * Throws an exception if the sub-value is an array (eg `array('!=' => array())`). It needs to just be a string, |
||
361 | * of either comma-separated-values, or a JSON array. |
||
362 | * @since 4.9.72.p |
||
363 | * @param $sub_array_key |
||
364 | * @param $sub_array_value |
||
365 | * @throws RestException |
||
366 | */ |
||
367 | private function assertSubValueIsntArray($sub_array_key, $sub_array_value) |
||
386 | |||
387 | /** |
||
388 | * Determines if the sub-array key is an operator taking 3 or more operators. |
||
389 | * @since 4.9.72.p |
||
390 | * @param $sub_array_key |
||
391 | * @return boolean |
||
392 | */ |
||
393 | private function subArrayKeyIsNonBinaryOperator($sub_array_key) |
||
403 | |||
404 | /** |
||
405 | * Given that the $sub_array_key is a string, checks if it's an operator taking only 1 argument. |
||
406 | * @since 4.9.72.p |
||
407 | * @param string $sub_array_key |
||
408 | * @return boolean |
||
409 | */ |
||
410 | private function subArrayKeyIsUnaryOperator($sub_array_key) |
||
417 | |||
418 | /** |
||
419 | * Parses the $sub_array_value string into an array (given it could either be a comma-separated-list or a JSON |
||
420 | * array). eg `"1,2,3"` or `"[1,2,3]"` into `array(1,2,3)`. |
||
421 | * @since 4.9.72.p |
||
422 | * @param $sub_array_value |
||
423 | * @return array|mixed|object |
||
424 | */ |
||
425 | private function extractQuickStyleSpecifiedOperatorValue($sub_array_value) |
||
442 | |||
443 | /** |
||
444 | * Throws an exception if the value isn't a simplified specified operator (only called when we expect that). |
||
445 | * @since 4.9.72.p |
||
446 | * @throws RestException |
||
447 | */ |
||
448 | View Code Duplication | private function assertSimplifiedSpecifiedOperator() |
|
467 | |||
468 | /** |
||
469 | * If query_param_value were in the simplified specific operator structure, change it into the legacy structure. |
||
470 | * @since 4.9.72.p |
||
471 | * @throws RestException |
||
472 | */ |
||
473 | private function transformSimplifiedSpecifiedOperatorSyntaxIntoStandardSyntax() |
||
494 | |||
495 | /** |
||
496 | * Returns true is the value is an array using the legacy structure to specify the operator. Eg `array('!=',123)`. |
||
497 | * @since 4.9.72.p |
||
498 | * @return boolean |
||
499 | */ |
||
500 | private function valueIsLegacySpecifiedOperator() |
||
507 | |||
508 | /** |
||
509 | * Returns true if the value specified operator accepts arbitrary number of arguments, like "IN". |
||
510 | * @since 4.9.72.p |
||
511 | * @param $operator |
||
512 | * @return boolean |
||
513 | */ |
||
514 | View Code Duplication | private function operatorIsNAry($operator) |
|
525 | |||
526 | /** |
||
527 | * Returns true if the operator accepts 3 arguments (eg "BETWEEN"). |
||
528 | * So we're looking for a value that looks like |
||
529 | * `array('BETWEEN', array('2015-01-01T00:00:00', '2016-01-01T00:00:00'))`. |
||
530 | * @since 4.9.72.p |
||
531 | * @param $operator |
||
532 | * @return boolean |
||
533 | */ |
||
534 | private function operatorIsTernary($operator) |
||
544 | |||
545 | /** |
||
546 | * Returns true if the operator is a similar to LIKE, indicating the value may have wildcards we should leave alone. |
||
547 | * @since 4.9.72.p |
||
548 | * @param $operator |
||
549 | * @return boolean |
||
550 | */ |
||
551 | View Code Duplication | private function operatorIsLike($operator) |
|
558 | |||
559 | /** |
||
560 | * Returns true if the operator only takes one argument (eg it's like `IS NULL`). |
||
561 | * @since 4.9.72.p |
||
562 | * @param $operator |
||
563 | * @return boolean |
||
564 | */ |
||
565 | private function operatorIsUnary($operator) |
||
571 | |||
572 | /** |
||
573 | * Returns true if the operator specified is a binary opeator (eg `=`, `!=`) |
||
574 | * @since 4.9.72.p |
||
575 | * @param $operator |
||
576 | * @return boolean |
||
577 | */ |
||
578 | private function operatorisBinary($operator) |
||
594 | |||
595 | /** |
||
596 | * If we're debugging, throws an exception saying that the wrong number of arguments was provided. |
||
597 | * @since 4.9.72.p |
||
598 | * @param $operator |
||
599 | * @throws RestException |
||
600 | */ |
||
601 | private function throwWrongNumberOfArgsExceptionIfDebugging($operator) |
||
619 | |||
620 | /** |
||
621 | * Wrapper for ModelDataTranslator::prepareFieldValuesFromJson(), just a tad more DRY. |
||
622 | * @since 4.9.72.p |
||
623 | * @param $value |
||
624 | * @return mixed |
||
625 | * @throws RestException |
||
626 | */ |
||
627 | private function prepareValuesFromJson($value) |
||
636 | |||
637 | /** |
||
638 | * Throws an exception if an invalid operator was specified and we're debugging. |
||
639 | * @since 4.9.72.p |
||
640 | * @throws RestException |
||
641 | */ |
||
642 | View Code Duplication | private function throwInvalidOperatorExceptionIfDebugging() |
|
662 | |||
663 | /** |
||
664 | * Returns true if the query_param_key was a logic query parameter, eg `OR`, `AND`, `NOT`, `OR*`, etc. |
||
665 | * @since 4.9.72.p |
||
666 | * @return boolean |
||
667 | */ |
||
668 | private function isLogicQueryParam() |
||
672 | |||
673 | |||
674 | /** |
||
675 | * If the query param isn't for a field, it must be a nested query parameter which requires different logic. |
||
676 | * @since 4.9.72.p |
||
677 | * @return array |
||
678 | * @throws DomainException |
||
679 | * @throws EE_Error |
||
680 | * @throws RestException |
||
681 | * @throws InvalidDataTypeException |
||
682 | * @throws InvalidInterfaceException |
||
683 | * @throws InvalidArgumentException |
||
684 | */ |
||
685 | public function determineNestedConditionQueryParameters() |
||
729 | } |
||
730 | // End of file RestQueryParamMetadata.php |
||
732 |
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: