Destramic Posted August 5, 2018 Share Posted August 5, 2018 hey guys im trying to build custom http error pages into my framework but after days of thinking, i must admit defeat and ask for some help. ok so if i try to access a route which doesnt exist my dispatcher will send a 404 error in the send_404_error() method were http_reponse_code(404) is set and then it'll dispatch to the error controller (no problem). dispatcher: <?php namespace MVC\Dispatcher; use HTTP\Request; use HTTP\Response; use Exception; class Dispatcher { private $request; private $config; private $class; private $controller; private $method; private $dispatchable = null; public function __construct(Request $request, $config) { $this->request = $request; $this->config = $config; } public function dispatch(array $parameters) { if ($this->dispatchable) { $this->request->set_controller($this->controller); $this->request->set_parameters($parameters); $controller = new $this->class($this->request); $this->reset(); return call_user_func_array(array($controller, $this->method), $parameters); } else if ($this->dispatchable == false) { return $this->send_404_error(); } return false; } public function is_dispatchable(string $controller) { $this->format_controller($controller); if (!class_exists($this->class) || !method_exists($this->class, $this->method)) { return false; } return $this->dispatchable = true; } private function format_controller(string $controller) { if (substr_count($controller, ':') != 2) { throw new Exception('Dispatcher: Invalid controller format.'); } $this->controller = $controller; list ($module, $controller, $method) = explode(':', $controller); $this->class = $module . '\Controller\\' . $controller; $this->method = $method; } public function send_404_error() { if (isset($this->config->framework['http_error_controller'])) { $error_controller = $this->config->framework['http_error_controller']; if ($this->is_dispatchable($error_controller)) { (new Response)->set_response_code('404'); return $this->dispatch(array( 'status_code' => 404 )); } else { throw new Exception('Dispatcher: HTTP Error undispatchable.'); } } return false; } public function reset() { $this->class = null; $this->controller = null; $this->methid = null; $this->dispatchable = false; } } question what happens if i want to send a http 404 error in any of my controllers? <?php namespace Custom\Controller; class Test extends Controller { public function test($status_code) { // user not authed! (new Response)->set_response_code(404)->send(); } } when the 404 response is sent, i could check in send() method that the http_repsone_code() is 200, if not then dispatch to the error controller. if ($this->get_response_code() !== 200) { return (new Front_Controller($config))->run('custom:error:http_error', 404); } return $this->content all that being said there is still a http_repsone_code(404) after that snippit is run and the response will keep looping because the repsone code isn't 200. response: <?php namespace HTTP; use HTTP\Response\Header; class Response { private $content; public function __construct($content = null) { $this->set_content($content); } public function set_content(string $content = null) { $this->content = $content; } public function set_response_code(int $status_code) { http_response_code($status_code); } public function get_response_code() { return http_response_code(); } public function headers() { return new Header; } public function send() { echo $this->content; } } can someone please talk some sence into my on how i should handle http reponse codes in my framework. thank you. Quote Link to comment Share on other sites More sharing options...
requinix Posted August 5, 2018 Share Posted August 5, 2018 You're creating new objects all over. That's weird. There should be exactly one request and one response at a time, and they should be created early on in the process. Probably one FrontController too. Look through all your code for any time you're using "new" and decide whether you really do want a brand new instance every time - if you do then you're probably singleton-ing stuff that should not be; prepare your framework so that you could run multiple request/response pairs in one execution, like if you wanted to do an internal redirect or for testing. This normally works like 1. First file (eg, index.php) gets/creates the FrontController 2. Same file also does some sort of Request::getFromCurrentRequest to get a request object 3. Same file, or the FrontController, or the Request, creates a corresponding response object 4. FrontController runs the request The Request and the Response are both available to the code that needs them. Want to change to a 404? Do that on the Response. The Response class shouldn't take any action until it's actually ready to return the response. Don't http_response_code() yet. Track it internally, default 200, and let it be changed by whatever wants to change it. When the request completes and it's time for the response to be sent, then it uses http_response_code() or whatever. 1 Quote Link to comment Share on other sites More sharing options...
Destramic Posted August 6, 2018 Author Share Posted August 6, 2018 brilliant i understand how it should work now....i'll change my code around so the request and response get sent from front controller to my controller. that way i'll be able to run this code in my front controller if dispatched. $controller = $this->dispatcher->dispatch(); $response = $controller->get_repsonse(); if ($reponse->get_response_code !== 200) { // deal with respsone code! { else { $response->send(): } also should i send my view through my front controller to controller like request and response?...or initialize my view in my asbtract controller? thank you for your help! Quote Link to comment Share on other sites More sharing options...
requinix Posted August 6, 2018 Share Posted August 6, 2018 I wouldn't do the !==200 stuff either. The Response is responsible for sending the response. It should be the one that decides how to handle non-200s. If it decides that, say, a 404 should go through some Request::send_404_error method then it should be the one doing that. Similarly, the front controller is not responsible for the view either. Its job was to build or otherwise get a Request and possibly Response, then route that to a controller. That's all. The rest of the framework takes over from there. Well, it doesn't have to be that strict. It could find the controller, run the controller, get a view as a return value, and pass that to the response. Two lines of code. Point is that it does very little work and contains very little logic once the request was routed. Either way, the controller is how the view gets to be in the response. The Response should want some sort of actionable thing it can use to actually, you know, respond. So the controller would figure out what View to use and that gets to the Response, which does its checks for errors or whatever, sends out initial headers, then runs the ActionableThing which does what it wants to do (like send maybe a couple more headers and move on to the response body). Quote Link to comment Share on other sites More sharing options...
Destramic Posted August 9, 2018 Author Share Posted August 9, 2018 requinix thank you for your time in breaking down how things should work, as it's all alot clearer to me now and i'll be sure to adopt these changes into my framework. thanks again. Quote Link to comment Share on other sites More sharing options...
Recommended Posts
Join the conversation
You can post now and register later. If you have an account, sign in now to post with your account.