Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update Client to handle response data in a new method #63

Closed
thinkingserious opened this issue Oct 30, 2017 · 1 comment · Fixed by #79
Closed

Update Client to handle response data in a new method #63

thinkingserious opened this issue Oct 30, 2017 · 1 comment · Fixed by #79
Labels
difficulty: easy fix is easy in difficulty

Comments

@thinkingserious
Copy link
Contributor

thinkingserious commented Oct 30, 2017

Method makeRequest has 33 lines of code (exceeds 25 allowed). Consider refactoring.

https://codeclimate.com/github/sendgrid/php-http-client/lib/Client.php#issue_59f3f044c7fa44000100001a

Is it possible to move all of this to another method in the class?

$headerSize = curl_getinfo($curl, CURLINFO_HEADER_SIZE);
        $statusCode = curl_getinfo($curl, CURLINFO_HTTP_CODE);
 
        $responseBody = substr($response, $headerSize);
        $responseHeaders = substr($response, 0, $headerSize);
 
        $responseHeaders = explode("\n", $responseHeaders);
        $responseHeaders = array_map('trim', $responseHeaders);
 
        curl_close($curl);
     
        $response = new Response($statusCode, $responseBody, $responseHeaders);

Also, can the retry logic be moved to another method?

       if ($statusCode == 429 && $retryOnLimit) {
            $headers = $response->headers(true);
            $sleepDurations = $headers['X-Ratelimit-Reset'] - time();
            sleep($sleepDurations > 0 ? $sleepDurations : 0);
            return $this->makeRequest($method, $url, $body, $headers, false);
        }
@mbernier mbernier changed the title Fix "method_lines" issue in lib/Client.php Update Client to handle response data in a different class method Oct 30, 2017
@mbernier mbernier changed the title Update Client to handle response data in a different class method Update Client to handle response data in a new method Oct 30, 2017
@mbernier mbernier added difficulty: easy fix is easy in difficulty good first issue labels Oct 30, 2017
@michaeljdennis
Copy link
Contributor

I'll handle this one.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
difficulty: easy fix is easy in difficulty
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants