Complex classes like ErrorHandler 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 ErrorHandler, and based on these observations, apply Extract Interface, too.
| 1 | <?php |
||
| 23 | class ErrorHandler |
||
| 24 | { |
||
| 25 | // General return categories: OK or failure (By default, failures are most probably API rejections) |
||
| 26 | const OK = 1 << 0; |
||
| 27 | const FAILURE = 1 << 1; |
||
| 28 | |||
| 29 | // "Network" failure "subcategory" |
||
| 30 | const NET_FAILURE = 1 << 2; |
||
| 31 | |||
| 32 | // Kind of failure "tag". Some network or API errors are judged "soft" or "temporary". |
||
| 33 | // It's up to the callee to test SOFT_FAILURE congruence and make use of this information if desired. |
||
| 34 | const SOFT_FAILURE = 1 << 3; |
||
| 35 | |||
| 36 | // successes |
||
| 37 | const INSERTED = 1 << 6; |
||
| 38 | const UPDATED = 1 << 7; |
||
| 39 | const DELETED = 1 << 8; |
||
| 40 | |||
| 41 | // api rejections |
||
| 42 | const INVALID_RES = 1 << 10; // usually from a programmer error |
||
| 43 | const INVALID_API_KEY = 1 << 11; // idem |
||
| 44 | const ALREADY_EXISTS = 1 << 12; |
||
| 45 | const COMPLIANCE = 1 << 13; |
||
| 46 | const FAKE = 1 << 14; |
||
| 47 | const INVALID_DOMAIN = 1 << 15; |
||
| 48 | const CAPPED = 1 << 16; |
||
| 49 | const API_OTHER = 1 << 17; |
||
| 50 | const INVALID_ADDRESS = 1 << 18; // invalid ADDRESS merge field |
||
| 51 | |||
| 52 | // network failures |
||
| 53 | const NET_REQ_PROC = 1 << 20; // Neither permanent nor record-specific error. try again later. |
||
| 54 | const NET_TIMEOUT = 1 << 21; // curl: Operation timed out |
||
| 55 | const NET_NO_JSON = 1 << 22; // non-JSON response |
||
| 56 | const NET_UNKNOWN = 1 << 23; |
||
| 57 | const UNKNOWN_ERROR = 1 << 24; |
||
| 58 | |||
| 59 | /** |
||
| 60 | * A couple of shortcuts to directly return a specific error of a given category |
||
| 61 | */ |
||
| 62 | private function ok(int $r, array $a) |
||
| 63 | { |
||
| 64 | return $this->log($r | self::OK, $a); |
||
| 65 | } |
||
| 66 | private function fail(int $r, array $a) |
||
| 67 | { |
||
| 68 | return $this->log($r | self::FAILURE, $a); |
||
| 69 | } |
||
| 70 | private function netfail(int $r, array $a) |
||
| 71 | { |
||
| 72 | return $this->fail($r | self::NET_FAILURE, $a); |
||
| 73 | } |
||
| 74 | |||
| 75 | /** |
||
| 76 | * A list of well-known failures. |
||
| 77 | * They are tested in order. Put more restrictive first |
||
| 78 | * At the step where these are probed, |
||
| 79 | * response is certainly a *failure*, as such all below return codes must be also be OR-ed with self::FAILURE |
||
| 80 | */ |
||
| 81 | const CODES = [ |
||
| 82 | '400' => [ |
||
| 83 | // See https://developer.mailchimp.com/documentation/mailchimp/guides/error-glossary/ for categories |
||
| 84 | // 'BadRequest', // Your request could not be processed. |
||
| 85 | // 'InvalidAction', |
||
| 86 | // 'InvalidResource', // The resource submitted could not be validated. |
||
| 87 | // 'JSONParseError', |
||
| 88 | ['code' => self::ALREADY_EXISTS, |
||
| 89 | 'tests' => [ 'title' => 'Member Exists', |
||
| 90 | 'detail' => 'is already a list member']], |
||
| 91 | |||
| 92 | ['code' => self::COMPLIANCE, |
||
| 93 | 'tests' => ['title' => 'Member In Compliance State', |
||
| 94 | 'detail' => 'is in a compliance state due to unsubscribe, bounce, or compliance review']], |
||
| 95 | |||
| 96 | ['code' => self::INVALID_API_KEY | self::SOFT_FAILURE, |
||
| 97 | 'tests' => ['title' => 'API Key Invalid']], |
||
| 98 | |||
| 99 | ['code' => self::FAKE, |
||
| 100 | 'tests' => ['title' => 'Invalid Resource', |
||
| 101 | 'detail' => 'looks fake or invalid']], |
||
| 102 | |||
| 103 | ['code' => self::INVALID_ADDRESS, |
||
| 104 | 'tests' => ['title' => 'Invalid Resource', |
||
| 105 | 'detail' => 'Your merge fields were invalid', |
||
| 106 | 'errors' => 'Please enter a complete address']], |
||
| 107 | |||
| 108 | ['code' => self::INVALID_DOMAIN, |
||
| 109 | 'tests' => ['title' => 'Invalid Resource', |
||
| 110 | 'detail' => 'domain portion of the email address is invalid']], |
||
| 111 | |||
| 112 | ['code' => self::CAPPED, |
||
| 113 | 'tests' => ['title' => 'Invalid Resource', |
||
| 114 | 'detail' => 'has signed up to a lot of lists very recently']], |
||
| 115 | |||
| 116 | ['code' => self::INVALID_RES | self::SOFT_FAILURE, |
||
| 117 | 'tests' => ['title' => 'Invalid Resource', |
||
| 118 | 'detail' => 'The resource submitted could not be validated']], |
||
| 119 | ], |
||
| 120 | /* |
||
| 121 | '401' => [ |
||
| 122 | 'APIKeyMissing', |
||
| 123 | 'APIKeyInvalid', |
||
| 124 | ], |
||
| 125 | '403' => [ |
||
| 126 | 'Forbidden', |
||
| 127 | 'UserDisabled', |
||
| 128 | 'WrongDatacenter' |
||
| 129 | ], |
||
| 130 | '404' => [ |
||
| 131 | 'ResourceNotFound' |
||
| 132 | ] |
||
| 133 | '405' => [ |
||
| 134 | 'MethodNotAllowed' |
||
| 135 | ], |
||
| 136 | '414' => [ |
||
| 137 | 'ResourceNestingTooDeep' |
||
| 138 | ], |
||
| 139 | '422' => [ |
||
| 140 | 'InvalidMethodOverride' |
||
| 141 | ], |
||
| 142 | '429' => [ |
||
| 143 | 'TooManyRequests' |
||
| 144 | ], |
||
| 145 | '500' => [ |
||
| 146 | 'InternalServerError' |
||
| 147 | ], |
||
| 148 | '503' => [ |
||
| 149 | 'ComplianceRelated' |
||
| 150 | ] |
||
| 151 | */ |
||
| 152 | ]; |
||
| 153 | |||
| 154 | /** |
||
| 155 | * Simple categorization, exclusively used for logging. |
||
| 156 | */ |
||
| 157 | const ERROR_CLASSES = [ |
||
| 158 | self::COMPLIANCE => 'compliance-state', |
||
| 159 | self::FAKE => 'fake', |
||
| 160 | self::INVALID_DOMAIN => 'invalid-domain', |
||
| 161 | self::CAPPED => 'capped' |
||
| 162 | ]; |
||
| 163 | |||
| 164 | // Unused |
||
| 165 | const MC_RETURN_CODES = [ ['code' => -100, 'name' => 'ValidationError'], |
||
| 166 | ['code' => -99, 'name' => 'List_RoleEmailMember'], |
||
| 167 | ['code' => 212, 'name' => 'List_InvalidUnsubMember'], |
||
| 168 | ['code' => 213, 'name' => 'List_InvalidBounceMember'], |
||
| 169 | ['code' => 234, 'name' => 'List_ThrottledRecipient'] ]; |
||
| 170 | |||
| 171 | protected $logger; |
||
| 172 | |||
| 173 | public function __construct(?\Psr\Log\LoggerInterface $logger) |
||
| 174 | { |
||
| 175 | if ($logger) { |
||
| 176 | $this->logger = $logger; |
||
| 177 | } else { |
||
| 178 | $this->logger = new class |
||
| 179 | { |
||
| 180 | public function __call($name, $args) |
||
| 184 | }; |
||
| 185 | } |
||
| 186 | } |
||
| 187 | |||
| 188 | /** |
||
| 189 | * @param MailChimp $mailchimp A MailChimp class. |
||
| 190 | * @param array $json_response The MailChimp response. |
||
| 191 | * @param string $method The method used in the request. |
||
| 192 | * @param array $loginfo Additional parameter(s) to pass as logging variables. |
||
| 193 | * |
||
| 194 | * Example: |
||
| 195 | * |
||
| 196 | * $response = $mc->post($url, $some_data); |
||
| 197 | * return $errorHandler->errno($mc, $response, 'POST', ['id' => $request_id]) |
||
| 198 | * |
||
| 199 | */ |
||
| 200 | public function errno(MailChimp $mailchimp, array $json_response, string $method, array $loginfo = []) |
||
| 290 | |||
| 291 | /** |
||
| 292 | * Logging API errors into the database is wrong. |
||
| 293 | * It's the role of our logger to perform well enough |
||
| 294 | */ |
||
| 295 | public function log(int $code, array $logargs) |
||
| 376 | |||
| 377 | /** |
||
| 378 | * Monolog default LineFormatter appends the whole context at the end of the log line. |
||
| 379 | * Even if elements are used as message placeholders. This (PSR-3) interpolation function |
||
| 380 | * avoids this. |
||
| 381 | */ |
||
| 382 | private static function interpolate(string $message, array $context = []) |
||
| 398 | |||
| 399 | public static function obfuscateRequest($request) |
||
| 421 | } |
||
| 422 |
This check looks for variable assignements that are either overwritten by other assignments or where the variable is not used subsequently.
Both the
$myVarassignment in line 1 and the$higherassignment in line 2 are dead. The first because$myVaris never used and the second because$higheris always overwritten for every possible time line.