Critique abstract class for handling GET and POST requests?
I'm only interested in handling GET or POST requests, so I designed this abstract class to determine which request has been made and to subsequently call the appropriate function. I would really appreciate feedback on this. Thanks!
PS I think this should be a community wiki, but I'm not sure how to set it as that.
abstract class AHttpRequestHandler
{
public function handleRequest()
{
if($_SERVER['REQUEST_METHOD'] == 'POST') {
$this->handlePostRequest();
} else if($_SERVER['REQUEST_METHOD'] == 'GET') {
$this->handleGetRequest();
} else {
$this->handleIllegalRequest();
}
}
abstract protected function handleGetRequest();
abstract protected function handlePostRequest();
protected function handleIllegalRequest()
{
throw new Exception('Illegal request detected in HttpRequestHandler::handleIllegalRequest().');
}
}
In response to comments:
I will only be handling one or the other (GET or POST), never both at the same time. Either an HTML form will be submitted via POST, or a redirect will be made with a query string, which will be a GET request. I am not familiar with how a mixed request could be made (both GET and POST), but since this is a personal project I have control over whether it happens or not.
I use the AHttpRequestHandler
class (above) by implementing the handleGetRequest()
and handlePostRequest()
methods in a sub-class, which is and abstract controller, AController
. Then, for each page of my CMS, I create a sub-class of AController
, such as ImageUpload
or ImageDetailsEditor
. I can provide more details if it will help.
Here are the AController
, Controller
, and View
classes:
AController
abstract class AController extends AHttpRequestHandler
{
protected $view;
public function __construct()
{
$this->handleRequest();
}开发者_JAVA百科
protected function handleGetRequest()
{
throw new Exception('handleGetRequest not yet implemented.');
}
protected function handlePostRequest()
{
throw new Exception('handlePostRequest not yet implemented.');
}
abstract protected function initView();
}
Controller
class Controller extends AController
{
protected $content;
public function __construct()
{
$this->view = new View();
parent::__construct();
}
protected function handleGetRequest()
{
$this->content = 'GET Request';
$this->initView();
}
protected function handlePostRequest()
{
$this->content = 'POST Request';
$this->initView();
}
protected function initView()
{
$this->view->content = $this->content;
$this->view->display();
}
}
View
//An over-simplified view for example use only
class View
{
public $content;
public function display()
{
echo "<p>$this->content</p>";
}
}
The actual use:
require_once 'Controller.php';
$controller = new Controller();
First of all you can make a GET request and a POST request in the same time. Think of a form that you post but the url has some variables in the query ( get ).
1.I don't understand the need for such a class but the first thing you could do is make two separate classes for post and get that extend the AHttpRequestHandler class. That way you only need an abstract function handleRequest that you will implement in the child classes.
2.You should apply "Intention Revealing Names". Your class should be RequestHandler and your methods should not contain Request in them. You know that from the class name.
3.Think about this: you might need to handle the post request in one controller. So you will have to add the second abstract method each time just to respect the abstract class.
4.You should not make circular calls between classes ( The Hollywood principle ). handleRequest is called from the child class, and then the parent calls handleGetRequest or handlePostRequest from the child.
Like I said, you are the developer, you know each controller what will use:POST or GET ( what about COOKIEs? ), so you can handle them at controller level without the need to extra classes just for the sake of it.
- see ref
- see ref
- see ref
- see ref
- And the Controller should receive a request (command), not extend the request to keep things apart. Have no catch phrase for that, perhaps seperation of concerns. That's an extension to 1. above but only if you really need a request object.
Having an abstract class for requests is a good idea and it is there in all frameworks. But I dont think its good to extend this class by all controllers. A better solution will be to separate this to two, an abstract request class and base controller class. In request class you can have methods to identify whether it is a get request or post request, like
class Request{
public function isPost() {
return ($_SERVER['REQUEST_METHOD'] == 'POST');
}
public function isGet() {
return ($_SERVER['REQUEST_METHOD'] == 'GET');
}
}
Also we will have a base controller class with at least the following options
class Controller
{
public $request;
public function __construct() {
$this->setRequest(new Request());
}
public function setRequest(Request $request) {
$this->request = $request;
}
}
All our client controllers will extend the base controller as usual. The advantage of this method is client controllers will have the freedom to determine the request type. if they want to make use of GET and POST request at a time, that also will be possible. The above given is of course an incomplete one. You need to add more methods to the base classes or not is your choice.
精彩评论