# PHP HTTP request class

I would like to get you opinion on this class, which is used to send HTTP requests (at that moment only POST method).

It is working ok, but I would like to get response on maintainability, reusability, security, code formatting, etc...

Class HttpRequest{

private $POST = 'POST'; private$PUT = 'PUT';
private $GET = 'GET'; private$DELETE = 'DELETE';
private $PATCH = 'PATCH'; private$body;
private $options; private$handle;
private $httpCode; private$response;

public function __construct(){}

/**
* send post request
* @param url
* @param options
* @param body
* @return json object
*/
public function post($url,$header, $options,$body){
if(!$this->handle || get_resource_type($this->handle) == "Unknown"){
$this->handle = curl_init(); } curl_setopt($this->handle, CURLOPT_URL, $url); curl_setopt($this->handle, CURLOPT_HTTPHEADER, $header); curl_setopt_array($this->handle, $options); // curl_setopt($this->handle, CURLOPT_SSL_VERIFYPEER, true);
// curl_setopt($this->handle, CURLOPT_SSL_VERIFYHOST, 0); curl_setopt($this->handle, CURLOPT_RETURNTRANSFER, true);
curl_setopt($this->handle, CURLOPT_CUSTOMREQUEST,$this->POST);
curl_setopt($this->handle, CURLOPT_POSTFIELDS,$body);
$this->response = curl_exec($this->handle);
$this->httpCode = curl_getinfo($this->handle, CURLINFO_HTTP_CODE);
curl_close($this->handle); return$this->response;
}

public function getResponse(){
return $this->response; } public function getHttpCode(){ return$this->httpCode;
}

/**
* send get request
* @param url
* @param options
*/
public function get($url,$header=array(), $options=array()){ /** * @todo * implemets this method */ } /** * send patch request */ public function patch(){ /** * @todo * implemets this method */ } /** * send delete request */ public function delete(){ /** * @todo * implemets this method */ } /** * send put request */ public function put(){ /** * @todo * implemets this method */ } } ?> • Instead of defining POST/GET as variables, define constants. – hjpotter92 Jan 19 at 12:38 ## 3 Answers Use class constants to store values that will remain the same and are unchangeable Your HTTP method verbs are better off set as class constants than class properties since they will remain the same and are unchangeable. (i.e. constant) private$POST = 'POST';
private $PUT = 'PUT'; private$GET = 'GET';
private $DELETE = 'DELETE'; private$PATCH = 'PATCH';

becomes

const POST = 'POST';
const PUT = 'PUT';
const GET = 'GET';
const DELETE = 'DELETE';
const PATCH = 'PATCH';

You can then refer to them using self

self::POST

No need for an empty constructor

If your contractor doesn't have any code in it you could, and should omit it. It's just noise otherwise. You can always add it if the constructor definition changes.

Not just class methods should have well written docblock comments, so should your class properties.

/**
* @var string The body of the HTTP request
*/
private $body; Follow PSR coding standards The PSR coding standards exist to ensure a high level of technical interoperability between shared PHP code. They also ensure conformity for projects with multiple developers. PSR-2 says that: Opening braces for classes MUST go on the next line, and closing braces MUST go on the next line after the body. and Opening braces for methods MUST go on the next line, and closing braces MUST go on the next line after the body. Class HttpRequest{ becomes Class HttpRequest { and public function post($url, $header,$options, $body){ becomes public function post($url, $header,$options, $body) { Use type hinting to variable types in class methods From the manual: Type declarations allow functions to require that parameters are of a certain type at call time. If the given value is of the incorrect type, then an error is generated To use your code as an example, you can declare that$url must be a string and $header and$options must be arrays. This will be enforced by PHP at run time and prevent obvious and non obvious errors from occurring.

public function get($url,$header=array(), $options=array()){ becomes public function get(string$url, array $header=[], array$options=[])
{

(I used shortened array syntax [] versus the more verbose array() for brevity.)

Use return type declarations

Just like you can enforce that class method parameters are a certain data type, you can also enforce the value returned by that method is a certain data type. Since all of your methods are stubs I'll pretend your HttpRequestget() method returns a Boolean to use as an example.

// curl_setopt($this->handle, CURLOPT_SSL_VERIFYHOST, 0); suppose you are going to comment and uncomment them when required. This is not how classes are used. Move these lines in a distinct method and call it when you have to bypass the SSL verification. ### Method chaining Given there could be several methods called in consequence, consider returning$this from methods. It will allow you to use the neat practice called "method chaining":

$request = new HttpRequest();$body = $request->setHeaders($headers)
->setNoSSLverification()
->post($url,$post_body)
->getResponse();

I like that you've tried to include some processing within object's methods, which is what objects should look like from OOP perspective, but you've chosen wrong case for it IMO.

### Naming & API

The class name and its API is confusing. Request itself shouldn't produce the response - the remote server does. Request is usually the data structure type which contains values that can be read from it, so that class representing remote server could fetch response based on it. The API of this server (that, in your case, encapsulates bunch of curl commands) would be:

$response =$httpServer->send($request); Your get(), post(), ...etc. methods are a good candidates for static factory methods (named constructors) producing concrete, possibly immutable request object, while replacing method parameter in primary constructor: class HttpRequest { public static function post($url, $header,$options, $body) { return self::__construct('POST',$url, $header,$options, $body); } public function __construct($method, $url,$header, $options,$body)
{
$this->method =$method;
$this->url =$url;
...
}

public function method(): string
{
return $this->method; } ... } ### Encapsulating I/O Since http structure is well defined and request contains complete information, its handler (remote server object) will be generic low level library - it won't need to change depending on where this request is sent. The layer (might be a single class) where you create this request, pass it to server object and process the response will be an adapter to abstract data port (google for ports and adapters). For example (ignore names here): class FacebookService implements UserDataGateway { public function __construct(APIConfig$config, RemoteServer $server) {$this->config = $config;$this->server = $server; } public function userData($userId): array
{
$request = ... //prepare request using$this->config and given $userId$response = $this->server->send($request);

return json_decode(\$response->body(), true);
}
...
}

By encapsulating http, the class that will get (and call) UserDataGateway might work with this FacebookService as well as with local SQL database or other remote service providing user data - it only needds to return it based on some user's id. It can also be tested in isolation from remote/database calls (this example likely returns plain view model data, so ther will be no logic to test).