Scrutinizer

PHP
1

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);
          }
2

Does Scrutinizer not provide a report on what it found?

3

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

4

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.

5

What is paths? and how you calculate it?

6

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.

7

How to refactor it to extract class as it suggests?

8

This topic was automatically closed 91 days after the last reply. New replies are no longer allowed.