HiveBrain v1.2.0
Get Started
← Back to all entries
patternphpMinor

Small PHP MVC Template

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
phptemplatesmallmvc

Problem

The following is a new question based on answers from here: Small PHP Viewer/Controller template

I have written a small MVC template library that I would like some critiques on.

The library is located here

If you are looking for where to start, check out the files in the smallFry/application directory.

I would really love to hear your critiques on:

  • Code quality



  • Code clarity



  • How to improve



  • Anything else that needs clarification expansion etc



I'm more interested in what I'm doing wrong than right.

Any opinions on the actual usefulness of the library are welcome.

Code examples (all of the code is equal, just on my last question I was asked for some snippets):

Bootstrap.php:

```
_session = new SessionManager(Config::get('APP_NAME'));
$this->_path = $this->readPath();
$this->_controller = $this->loadController();
$this->_template = new Template($this->_path, $this->_session, $this->_controller); //has destructor that controls it
$this->_controller->displayPage($this->_path->args); //run the page for the controller
$this->_template->renderTemplate(); //only render template after all is said and done
}

/**
*
* @return stdClass
*/
private function readPath(){
$path = isset($_SERVER["PATH_INFO"])?$_SERVER["PATH_INFO"]:'/'.Config::get('DEFAULT_CONTROLLER');

$path_info = explode("/",$path);
$page = (isset($path_info[2]) && strlen($path_info[2]) > 0)?$path_info[2]:'index';
list($page, $temp) = explode('.', $page) + array('index', null);
$args = array_slice($path_info, 3);
$controller = $path_info[1] ?: Config::get('DEFAULT_CONTROLLER');
return (object) array(
'path_info'=>$path_info,
'page'=>$page,
'args'=>$args,
'controller'=>$controller
);
}

/**
* @return AppController
*/
private function loadController(){
Config::set('page', $this->_pat

Solution

I think your code is at a point where you need to stop worrying about it. It's not perfect, but it's way past the point of anything obvious.

The one thing I really don't like is your Config class. I get it that static is heplful, but you are abusing it a bit. You could:

-
Consider turning it into a Singleton, if you are so inclined in static state. Singletons are evil, but an improvement upon your code. All static classes are virtually un-testable (in the context of unit tests) and I can only describe them as a procedural code masquerading as an object. It's a style you don't want to adopt.

-
Consider getting the configuration files from a configuration file, and not requiring the developer to hardcode them.

The simplest way would be ini files, PHP's support via parse_ini_file() is excellent. That way your configuration class could be used autonomously.

And, you seem to have ignored my advice on prefixing private members with an underscore. That hurts my feelings :)

Further suggestions (that are outside the scope of a code review):

  • PHPUnit is your best friend! Learn how to write unit tests, framework code especially should be 100% covered by unit tests.



  • PHP has support for interfaces and you should always program to an interface.



I honestly can't find anything else worth mentioning.

Context

StackExchange Code Review Q#7531, answer score: 4

Revisions (0)

No revisions yet.