Scrutinizer

I got rating C by scrutinizer for class method below. Please help to improve it to get level A.

          private function _call($url, $params = null, $headers = null, $method = "GET") 
          {
                 $ch = curl_init();
                 curl_setopt(/** @scrutinizer ignore-type */ $ch, CURLOPT_URL, $url);
                 curl_setopt(/** @scrutinizer ignore-type */ $ch, CURLOPT_FOLLOWLOCATION, false);
                 curl_setopt(/** @scrutinizer ignore-type */ $ch, CURLOPT_TIMEOUT, $this->_timeout);
                 curl_setopt(/** @scrutinizer ignore-type */ $ch, CURLOPT_RETURNTRANSFER, true);
                 curl_setopt(/** @scrutinizer ignore-type */ $ch, CURLOPT_SSL_VERIFYPEER, $this->_verify_ssl);
                 curl_setopt(/** @scrutinizer ignore-type */ $ch, CURLOPT_SSL_VERIFYHOST, $this->_verify_host);
                 curl_setopt(/** @scrutinizer ignore-type */ $ch, CURLOPT_HEADER, true);
                 curl_setopt(/** @scrutinizer ignore-type */ $ch, CURLINFO_HEADER_OUT, true);
                 if ($this->curlProxy) {  
                        curl_setopt(/** @scrutinizer ignore-type */ $ch, CURLOPT_PROXY, $this->curlProxy);  
                 }  
                 if ($this->_curl_callback) { 
                        call_user_func($this->_curl_callback, $ch, $params, $headers, $method); 
                 } 
                 switch (strtoupper($method)) { 
                        case 'PUT':
                        case 'PATCH':
                        case 'DELETE':
                                      curl_setopt(/** @scrutinizer ignore-type */ $ch, CURLOPT_CUSTOMREQUEST, strtoupper($method));
                                      curl_setopt(/** @scrutinizer ignore-type */ $ch, CURLOPT_POSTFIELDS, json_encode($params));
                        break;
                        case 'POST':
                                      curl_setopt(/** @scrutinizer ignore-type */ $ch, CURLOPT_POST, true);
                                      if ($this->json === true) {
                                             $params = json_encode($params);
                                      }
                                      curl_setopt(/** @scrutinizer ignore-type */ $ch, CURLOPT_POSTFIELDS, $params);
                        break;
                        case 'GET':
                                      curl_setopt(/** @scrutinizer ignore-type */ $ch, CURLOPT_HTTPGET, true);
                                      if (!empty($params)) {
                                             $url .= '?'.http_build_query($params);
                                             curl_setopt(/** @scrutinizer ignore-type */ $ch, CURLOPT_URL, $url);
                                      }
                        break;
                 }

                 $headers[] = $this->_api_key_var.$this->_api_key;
                 if ($this->json === true) {
                        $headers[] = 'Content-Type: application/json';
                 }
                 if ($this->accept === true) {
                        $headers[] = 'Accept: application/json';
                 }
              
                 curl_setopt(/** @scrutinizer ignore-type */ $ch, CURLOPT_HTTPHEADER, $headers);

                 $this->request['method'] = strtoupper($method);
                 $this->request['headers'] = $headers;
                 $this->request['params'] = $params;

                 $this->response = curl_exec(/** @scrutinizer ignore-type */ $ch);
                 if (curl_errno(/** @scrutinizer ignore-type */ $ch)) {
                        $this->curlErrno = curl_errno(/** @scrutinizer ignore-type */ $ch);
                        $this->curlError = curl_error(/** @scrutinizer ignore-type */ $ch);
                        curl_close(/** @scrutinizer ignore-type */ $ch);
                        return;
                 }
                 $this->curlInfo = curl_getinfo(/** @scrutinizer ignore-type */ $ch);
                 curl_close(/** @scrutinizer ignore-type */ $ch);
                 return new Result($this->_getBody(), $this->_getHeaders(), $this->curlInfo);
          }

Does Scrutinizer not provide a report on what it found?

Here is its report: https://scrutinizer-ci.com/g/phplicengine/bitly/code-structure/master/operation/PHPLicengine\Api\Api::_call

Right, and that report says the code is too long and too complex.
Not entirely sure how they get a value of 256 code execution paths, my math says there’s 128. But maybe i’m missing a conditional.

I don’t see anything particularly wrong here, but i suppose you could make separate functions for the different types of verbs to reduce complexity of this one.

What is paths? and how you calculate it?

Every time you have an IF, there’s two possible paths: Either the IF was true, or it wasnt. Those are different paths.

Your switch likewise does paths - 4, in fact:

switch (strtoupper($method)) {
case ‘PUT’:
case ‘PATCH’:
case ‘DELETE’:
curl_setopt(/** @scrutinizer ignore-type / $ch, CURLOPT_CUSTOMREQUEST, strtoupper($method));
curl_setopt(/
* @scrutinizer ignore-type / $ch, CURLOPT_POSTFIELDS, json_encode($params));
break;

case ‘POST’:
curl_setopt(/
* @scrutinizer ignore-type / $ch, CURLOPT_POST, true);
if ($this->json === true) {
$params = json_encode($params);
}
curl_setopt(/
* @scrutinizer ignore-type / $ch, CURLOPT_POSTFIELDS, $params);
break;

case ‘GET’:
curl_setopt(/
* @scrutinizer ignore-type / $ch, CURLOPT_HTTPGET, true);
if (!empty($params)) {
$url .= ‘?’.http_build_query($params);
curl_setopt(/
* @scrutinizer ignore-type */ $ch, CURLOPT_URL, $url);
}
break;
}

(“But m_hutley that’s only 3!”): Yes, but the fourth path is that it matched none of those, and fell into the Default condition, which is to do nothing.

There are 5 IF’s in your code, and a single switch with 4 states. 2*2*2*2*2*4 = 128.

And now that I look at it, I see the conditionals I missed. Two of those paths also have branching capability… so… there’s… another layer of complexity to the math that makes me think there’s actually more than 256.

there are 4 paths leading into the switch (the two initial IF’s); each of those may go down one of four paths. If they go down the second or third switch path, there’s another if; that means that the 4 going down those paths become 8 each; coming out of the switch you’d have 4+4+8+8 = 24 paths.
Then there’s a further 3 ifs, so those 24 become 48, then 96, then 198.

I don’t get 256, i get 198. I’m probably missing something though.

How to refactor it to extract class as it suggests?