patternphpMinor
PHP MVC class with controller and nested model
Viewed 0 times
mvcwithphpcontrollernestedandclassmodel
Problem
I had asked a first question about this class a while ago and got a few answers here which made me rewrite it completely.
I removed all statics and globals, added my variables as arguments for the constructor, and pass them again as reference through my constructor (for creating a controller from within a controller).
There is now a SVN with read-access: You have to do
from an empty folder, and to reach public_html from your browser.
```
namespace bbn\cls;
class mvc
{
use \bbn\traits\info;
private
$is_routed,
$is_controlled,
// The name of the controller
$dest,
// The path to the controller
$path,
// The controller file (with full path)
$controller,
// The mode of the output (dom, html, json, txt, xml...)
$mode;
public
// The data model
$data,
// The output object
$obj,
// The file extension of the view
$ext,
// The request sent to the server
$original_request,
// The first controller to be called at the top of the script
$original_controller,
// The list of used controllers with their corresponding request, so we don't have to look for them again.
$known_controllers = array(),
// The list of views which have been loaded. We keep their content in an array to not have to include the file again. This is useful for loops.
$loaded_views = array(),
// Mustage templating engine.
$mustache,
// Reference to $appui variable (that's my framework's name)
$appui,
// List of possible outputs with their according possible file extension
$outputs = array('dom'=>'html','html'=>'html','image'=>'jpg,jpeg,gif,png,svg','json'=>'json','pdf'=>'pdf','text'=>'txt','xml'=>'xml','js'=>'js'),
// List of possible and existing universal/default controller. First every ite
I removed all statics and globals, added my variables as arguments for the constructor, and pass them again as reference through my constructor (for creating a controller from within a controller).
There is now a SVN with read-access: You have to do
svn export https://babna.com/svn/_appui_pub ./public_htmlfrom an empty folder, and to reach public_html from your browser.
```
namespace bbn\cls;
class mvc
{
use \bbn\traits\info;
private
$is_routed,
$is_controlled,
// The name of the controller
$dest,
// The path to the controller
$path,
// The controller file (with full path)
$controller,
// The mode of the output (dom, html, json, txt, xml...)
$mode;
public
// The data model
$data,
// The output object
$obj,
// The file extension of the view
$ext,
// The request sent to the server
$original_request,
// The first controller to be called at the top of the script
$original_controller,
// The list of used controllers with their corresponding request, so we don't have to look for them again.
$known_controllers = array(),
// The list of views which have been loaded. We keep their content in an array to not have to include the file again. This is useful for loops.
$loaded_views = array(),
// Mustage templating engine.
$mustache,
// Reference to $appui variable (that's my framework's name)
$appui,
// List of possible outputs with their according possible file extension
$outputs = array('dom'=>'html','html'=>'html','image'=>'jpg,jpeg,gif,png,svg','json'=>'json','pdf'=>'pdf','text'=>'txt','xml'=>'xml','js'=>'js'),
// List of possible and existing universal/default controller. First every ite
Solution
Don't like it too much.
Passing variables into functions as references always scare me because usually you cannot track anymore what's going on.
References communicate to me the following: "I expect this value to be not an object, and I expect the value to be changed inside, and the change also made intentionally effective on the outside."
This communication is clearly wrong in your case:
a) Either you get an object - then the reference is not needed because objects are ALWAYS passed as reference.
b) If you get a string, then you do nothing with the string but copy it to another variable.
Additionally there are some cases where you copy values as references that seem unnecessary to me.
Completely unrelated: Have you heard about autoloading? You seem to do some extensive stuff with paths to find your models for example, and then try to include them with a tricky wrapping.
I assume models are classes. If they are, then autoloading will load them the very time they are first needed because some code stumples upon it's classname. Check "PSR-0 autoloading" to get more info.
I just came across this part:
I would thing that inside
A final comment: I do not like the idea too much that you basically only have one controller class that does everything, and that your controllers are no real classes, but code fragments that act inside your single controller, stacking stuff.
And I miss type hinting very much in your code.
Passing variables into functions as references always scare me because usually you cannot track anymore what's going on.
__construct(&appui,...) is one such thing.References communicate to me the following: "I expect this value to be not an object, and I expect the value to be changed inside, and the change also made intentionally effective on the outside."
This communication is clearly wrong in your case:
a) Either you get an object - then the reference is not needed because objects are ALWAYS passed as reference.
b) If you get a string, then you do nothing with the string but copy it to another variable.
Additionally there are some cases where you copy values as references that seem unnecessary to me.
Completely unrelated: Have you heard about autoloading? You seem to do some extensive stuff with paths to find your models for example, and then try to include them with a tricky wrapping.
I assume models are classes. If they are, then autoloading will load them the very time they are first needed because some code stumples upon it's classname. Check "PSR-0 autoloading" to get more info.
I just came across this part:
// Processes the controller and checks whether it has been controlled or not.
public function check()
{
$this->process();
return $this->is_controlled;
}
// Returns the output object.
public function get()
{
if ($this->check() && $this->is_controlled)
return $this->obj;
return false;
}I would thing that inside
get() the if might be easier made like this:if ($this->check()) return $this->obj;A final comment: I do not like the idea too much that you basically only have one controller class that does everything, and that your controllers are no real classes, but code fragments that act inside your single controller, stacking stuff.
And I miss type hinting very much in your code.
Code Snippets
// Processes the controller and checks whether it has been controlled or not.
public function check()
{
$this->process();
return $this->is_controlled;
}
// Returns the output object.
public function get()
{
if ($this->check() && $this->is_controlled)
return $this->obj;
return false;
}if ($this->check()) return $this->obj;Context
StackExchange Code Review Q#19043, answer score: 2
Revisions (0)
No revisions yet.