5
\$\begingroup\$

Below you can find the constructor method and related fields of the main class of my REST API (note that the class contains more code than displayed) written in PHP 7.1. It does various things with the HTTP-request. It starts with getting the arguments and then it gets the method in which the request is made (like POST or GET). The request gets trimmed of tags and that gets saved back into the instance/member variable $this->request.

Now, to my best knowledge, $this->request in PHP always points to the HTTP-request when a HTTP-request has just been made. The code can only get here when a HTTP-request has been made and $this->method has been filled ($this->method = $_SERVER['REQUEST_METHOD'];).

In other words, $this->method should always exist, but I keep getting the 'Field declared dynamically' warning:

enter image description here

Is it okay to just ignore the warning or is there a best practice to deal with it?

Other tips to improve my code are welcome too. I've 'learned' PHP back in version 4 (I used it for uni once). I struggle with the OO concepts since introduced in a language where so much is allowed.

class BaseAPI {
 protected $method = '';
 protected $endpoint = '';
 protected $verb = ''; 
 protected $args = Array(); 
 public function __construct($request)
 {
 header("Access-Control-Allow-Orgin: *");
 header("Access-Control-Allow-Methods: *");
 header("Content-Type: application/json");
 $this->args = explode('/', rtrim($request, '/'));
 $this->endpoint = array_shift($this->args);
 if (array_key_exists(0, $this->args) && !is_numeric($this->args[0])) {
 $this->verb = array_shift($this->args);
 }
 $this->method = $_SERVER['REQUEST_METHOD'];
 if ($this->method == 'POST' && array_key_exists('HTTP_X_HTTP_METHOD', $_SERVER)) {
 if ($_SERVER['HTTP_X_HTTP_METHOD'] == 'DELETE') {
 $this->method = 'DELETE';
 } else if ($_SERVER['HTTP_X_HTTP_METHOD'] == 'PUT') {
 $this->method = 'PUT';
 } else {
 throw new IncorrectHeaderException("Unexpected Header");
 }
 }
 switch ($this->method) {
 case 'DELETE':
 case 'PUT':
 // don't do anything yet
 case 'POST':
 $this->request = $this->_cleanInput($_POST);
 break;
 case 'GET':
 $this->request = $this->_cleanInput($_GET);
 break;
 default:
 throw new IncorrectMethodException('Invalid Method');
 break;
 }
 }
 private function _cleanInput($data)
 {
 $cleaned_input = Array();
 if (is_array($data)) {
 foreach ($data as $k => $v) {
 $cleaned_input[$k] = $this->_cleanInput($v);
 }
 } else {
 $cleaned_input = trim(strip_tags($data));
 }
 return $cleaned_input;
 }
}
200_success
145k22 gold badges190 silver badges478 bronze badges
asked Nov 21, 2017 at 21:51
\$\endgroup\$

2 Answers 2

4
\$\begingroup\$

Now, to my best knowledge, $this->request in PHP always points to the HTTP-request when a HTTP-request has just been made

My guess is that you are thinking of the super-global $_REQUEST? That is not the same as $this->request (unless somehow that instance variable gets set to something else)...

Is it okay to just ignore the warning or is there a best practice to deal with it?

Declare the property, just as the other instance properties are declared (i.e. $method, $endpoint, $verb, etc.):

class BaseAPI {
 protected $request;

That way the IDE will know that the property should at least exist for each instance (though it might not be assigned a value until that switch statement).

Other Feedback

Switch statement cases:

switch ($this->method) {
 case 'DELETE':
 case 'PUT':
 // don't do anything yet

Those first cases are missing a break; so the PHP pre-processor will fall-through to the POST case.

header() calls in constructor

The three calls to header() in the constructor feels a little strange but then again it is difficult to know what the rest of your code looks like. Typically those would go in a method to render/send output.

answered Nov 21, 2017 at 22:03
\$\endgroup\$
5
  • \$\begingroup\$ Would that not make $this->request save into the property instead of into the request? \$\endgroup\$ Commented Nov 21, 2017 at 22:04
  • \$\begingroup\$ "instead of into the request"... I don't understand what that means... \$\endgroup\$ Commented Nov 21, 2017 at 22:09
  • \$\begingroup\$ @Belle- oh - perhaps you are thinking of the super-global $_REQUEST? \$\endgroup\$ Commented Nov 21, 2017 at 22:11
  • \$\begingroup\$ I'm afraid that adding it as a field would make $this->request no longer behave as expected. But I've just tried it and it works. Thanks! I think I should probably delete those switch cases, since I'm not using them at the moment. They're literally useless. \$\endgroup\$ Commented Nov 21, 2017 at 22:12
  • \$\begingroup\$ I think I might be thinking of the super-global $_REQUEST indeed. That seems logical. Some parts of php are magic to me, especially magic methods. \$\endgroup\$ Commented Nov 21, 2017 at 22:16
3
\$\begingroup\$

I don't recommend emitting these headers so early, and also not as a side-effect of a constructor:

header("Access-Control-Allow-Orgin: *");
header("Access-Control-Allow-Methods: *");
header("Content-Type: application/json");

Headers should be emitted when formulating the response, not before the request has been parsed. Once headers have been emitted, you cannot reliably change the HTTP status code. You might want to send a redirect or a "no content" response.

In particular, I would handle both of your thrown exceptions such that they return HTTP status 405, "Method not allowed'.

answered Nov 21, 2017 at 22:44
\$\endgroup\$
4
  • \$\begingroup\$ Thanks, that seems like some good tips. I've included some more code. Should the headers go into _response() like I've done now? \$\endgroup\$ Commented Nov 22, 2017 at 18:33
  • \$\begingroup\$ Please see What to do when someone answers . I have rolled back Rev 6 → 4. \$\endgroup\$ Commented Nov 22, 2017 at 18:50
  • \$\begingroup\$ Oh :( okay, that's too bad \$\endgroup\$ Commented Nov 22, 2017 at 18:53
  • \$\begingroup\$ You are welcome to ask a follow-up question as a separate post, though. \$\endgroup\$ Commented Nov 22, 2017 at 18:54

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.