2
\$\begingroup\$

I'm at the point where I am happy with the functionality of my code but I dislike how its written. It seems like I can be more modular with it but I can't seem to figure out how to improve on it without changing to much. I understand that I can probably merge both getMed Methods but they are end points in a URL which I rather keep separate. Any assistance will be greatly appreciated

private function getSIS($url, $session, Application $app)
{
    $response = "You don't have permission to access this!";

    if($app['security']->isGranted('ROLE_SUPER_ADMIN'))
    {
        //['fomservices']['sis'] is hard coded in my config dev/prod file.
        $client = new Client(['base_uri' => $app['fomservices']['sis'],]);
        //for testing
        $token = 123456;

        try {
            if (!empty($session)) {
                $response = $client->request('GET', $url.'/'.$session, [
                    'headers' => [
                        'token' => $token
                    ]
                ]);
            } else {
                $response = $client->request('GET', $url, [
                    'headers' => [
                        'token' => $token
                    ]
                ]);
            }
            return $response;
        } catch (ClientException $e) {
            $response = $e->getResponse();
            $responseBodyAsString = $response->getBody()->getContents();
            return new Response($responseBodyAsString, $response->getStatusCode());
        } catch (ServerException $e) {
            $response = $e->getResponse();
            $responseBodyAsString = $response->getBody()->getContents();
            return new Response($responseBodyAsString, $response->getStatusCode());
        } catch (BadResponseException $e){
            $response = $e->getResponse();
            $responseBodyAsString = $response->getBody()->getContents();
            return new Response($responseBodyAsString, $response->getStatusCode());
        }
    }

    return $response;
}

public function getMedAction(Application $app)
{
    $response = $this->getSIS('med', '', $app);
    if($response instanceof \GuzzleHttp\Psr7\Response)
        return $response->getBody();
    return new Response ($response);
}

public function getMedSessionAction($session, Application $app)
{
    $response = $this->getSIS('med', $session, $app);
    if($response instanceof \GuzzleHttp\Psr7\Response)
        return $response->getBody();
    return new Response ($response);
}
\$\endgroup\$
0
\$\begingroup\$

I was able to improve on my previous code by making sure all the return statements were returning the same type of response (String / StatusCode). This allowed the 20 methods calling this one must smaller since I didn't have to check what response contain.

private function getSIS($url, $session, Application $app)
{
    if($app['security']->isGranted('ROLE_SUPER_ADMIN'))
    {
        $client = new Client(['base_uri' => $app['fomservices']['sis']]);
        $token = 234561;

        // Calling the external API which multiple errors could occur.
        try {
            if (!empty($session)) {
                $response = $client->request('GET', $url.'/'.$session, [
                    'headers' => [
                        'token' => $token
                    ]
                ]);
            } else {
                $response = $client->request('GET', $url, [
                    'headers' => [
                        'token' => $token
                    ]
                ]);
            }
            return new Response($response->getBody(), 200);
        } catch (ClientException $e) {
            $response = $e->getResponse();
            return new Response($response->getBody()->getContents(), $response->getStatusCode());
        } catch (ServerException $e) {
            $response = $e->getResponse();
            return new Response($response->getBody()->getContents(), $response->getStatusCode());
        } catch (BadResponseException $e){
            $response = $e->getResponse();
            return new Response($response->getBody()->getContents(), $response->getStatusCode());
        }
    }

    return new Response("You don't have permission to access this!", 401);
}

public function getMedAction(Application $app)
{
    return $this->getSIS('med', '', $app);
}

public function getMedSessionAction($session, Application $app)
{
    return $this->getSIS('med', $session, $app);
}
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.