0
\$\begingroup\$

So I created a wrapper class to authenticate user via POST HTTP request.

    class Nucleolus
    {
        public const AUTHEN_USER_URL = "localhost:8000/authen/user";
    
        protected $ch;
    
        public function __construct(string $token)
        {
            $this->ch = curl_init();
            curl_setopt($this->ch, CURLOPT_URL, self::AUTHEN_USER_URL);
            curl_setopt($this->ch, CURLOPT_HTTPHEADER, [
                "Content-Type: application/json",
                "Authorization: Bearer " . $token,
            ]);
            curl_setopt($this->ch, CURLOPT_POST, 1);
            curl_setopt($this->ch, CURLOPT_RETURNTRANSFER, true);
        }
    
        public function authenticate(string $username, string $password, string $serverUrl = null)
        {
            $url = $serverUrl ? $serverUrl : $_SERVER['SERVER_NAME'];
            curl_setopt($this->ch, CURLOPT_POSTFIELDS, json_encode(compact('url', 'username', 'password')));
    
            return json_decode(curl_exec($this->ch), true);
        }
    
        public function __destruct()
        {
            curl_close($this->ch);
        }
    }

Then the other developers can do the following.

    $nucleolus = new Nucleolus($token);
    $result = $nucleolus->authenticate($username, $password);

and retrieve result as a PHP array.
Please review.

\$\endgroup\$

1 Answer 1

2
\$\begingroup\$

Let me be frank: this code is just a Frankenstein sewn from different unrelated parts. Basically here are two main problems:

  • sending HTTP requests is a distinct task and should be performed by another wrapper
  • splitting the main functionality between two methods just makes no sense. I do understand that you probably wanted to make use of constructor, but that's not how it is done

The HTTP request class

Basically there must be a class that does HTTP requests with methods to set particular settings, such as

  • setAuth(new BearerAuth($token))
  • setContentType()
  • setUrl()
  • etc

The error checking

Also, in this class, it is important to check for errors. Right now your code returns whatever is sent by the other party. But it is not necessarily would be JSON of a particular format. It could be HTML, JSON that contains some different data, an empty string, etc.

  • First of all the code must check for the curl_error. In case there is, it must throw an Exception
  • Then it must try to decode JSON. If it fails it must throw an Exception
  • Then it must check if the decoded data contains expected information. If it doesn't, it must throw an Exception
  • only then it must return the data

The actual class

And then there could be a class to interact with a particular API, something like this

class Nucleolus
{
    protected $transport;
    protected $host;

    public function __construct(string $host, string $token)
    {
        $this->transport = new HTTPTransport;
        $this->transport->setAuth(new BearerAuth($token));
        $this->transport->setContentType('application/json');
    }

    public function authenticate(string $username, string $password, string $serverUrl = null)
    {
        $this->transport->setUrl($this->host."/authen/user");
        $result = $this->transport->post([
            'url' => $serverUrl ? $serverUrl : $_SERVER['SERVER_NAME'], 
            'username' => $username, 
            'password' => $password,
        ]);
        $data = JSON::decode($result);
        if ($data === true)
            return true;
        } else {
            throw new NucleolusException("Incorrect data from auth: $result");
        }
    }
}

Destructor is not really needed as PHP will destruct everything itself.

\$\endgroup\$
5
  • \$\begingroup\$ thanks for your comment \$\endgroup\$ Mar 11, 2021 at 11:31
  • \$\begingroup\$ is there a way to do it in one file so that it's easy to distribute? \$\endgroup\$ Mar 11, 2021 at 11:35
  • \$\begingroup\$ What do you mean, "easily distribute"? Distribution is usually done through composer packages \$\endgroup\$ Mar 11, 2021 at 11:37
  • \$\begingroup\$ there are still people who don't use composer in the organization. \$\endgroup\$ Mar 11, 2021 at 11:42
  • \$\begingroup\$ Inside the organization a Concurrent Versions System such as git must be used for the code distribution \$\endgroup\$ Mar 11, 2021 at 11:45

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

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