patternphpMinor
Small PHP framework template library
Viewed 0 times
templatephpsmalllibraryframework
Problem
The following is a new question based on answers from here: Small MVC for PHP
I have written a small template library that I would like some critiques on, located here. If you are looking for where to start, check out the files in the
I would really love to hear your critiques on code quality, code clarity, and anything else that needs clarification expansion.
I'm more interested in what I'm doing wrong than right. The README is incomplete and there is a lot more functionality than what is documented.
Code examples (these might not have changed much since last year, but I was asked to add some code, so I am including the same two files as on my previous post):
Bootstrap.php:
```
CONFIG = $CONFIG;
$this->CONFIG->set('page_title', $this->CONFIG->get('DEFAULT_TITLE'));
$this->CONFIG->set('template', $this->CONFIG->get('DEFAULT_TEMPLATE'));
$this->_session = new SessionManager($this->CONFIG->get('APP_NAME'));
$this->_path = $this->readPath();
$this->_controller = $this->loadController();
$this->_template = new Template($this->_path, $this->_session, $this->CONFIG);
$this->_template->setController($this->_controller);
$this->_controller->setPage($this->_path->page);
$this->_controller->displayPage($this->_path->args); //run the page for the controller
$this->_template->renderTemplate($this->_path->args); //only render template after all is said and done
}
catch (\Exception $e) {
header("HTTP/1.1 404 Not Found");
exit;
}
}
/**
* @return \stdClass
*/
private function readPath(){
$default_controller = $this->CONFIG->get('DEFAULT_CONTROLLER');
$index = str_replace("/", "", \SmallFry\Config\INDEX);
//use strtok to remove the query string from the end fo the request
$request_uri = !isset($_SERVER["REQUEST_URI"]) ? "/" : strtok($_S
I have written a small template library that I would like some critiques on, located here. If you are looking for where to start, check out the files in the
lib directory.I would really love to hear your critiques on code quality, code clarity, and anything else that needs clarification expansion.
I'm more interested in what I'm doing wrong than right. The README is incomplete and there is a lot more functionality than what is documented.
Code examples (these might not have changed much since last year, but I was asked to add some code, so I am including the same two files as on my previous post):
Bootstrap.php:
```
CONFIG = $CONFIG;
$this->CONFIG->set('page_title', $this->CONFIG->get('DEFAULT_TITLE'));
$this->CONFIG->set('template', $this->CONFIG->get('DEFAULT_TEMPLATE'));
$this->_session = new SessionManager($this->CONFIG->get('APP_NAME'));
$this->_path = $this->readPath();
$this->_controller = $this->loadController();
$this->_template = new Template($this->_path, $this->_session, $this->CONFIG);
$this->_template->setController($this->_controller);
$this->_controller->setPage($this->_path->page);
$this->_controller->displayPage($this->_path->args); //run the page for the controller
$this->_template->renderTemplate($this->_path->args); //only render template after all is said and done
}
catch (\Exception $e) {
header("HTTP/1.1 404 Not Found");
exit;
}
}
/**
* @return \stdClass
*/
private function readPath(){
$default_controller = $this->CONFIG->get('DEFAULT_CONTROLLER');
$index = str_replace("/", "", \SmallFry\Config\INDEX);
//use strtok to remove the query string from the end fo the request
$request_uri = !isset($_SERVER["REQUEST_URI"]) ? "/" : strtok($_S
Solution
You have asked for critiques, so here it goes.
The comments for private variables in
Explain yourself in code
-
-
It is generally considered bad practice to have
-
Your function
The first rule of functions is that they should be small. The second rule is that they should be smaller than that.
-
I can see many hard-coded strings inside your functions, you may want to put them separately in a dedicated config class.
-
Writing style: You are using
-
It is a matter of personal preference, but I like to start private/protected function with underscore, to easily distinguish them from public ones.
- A readability remark:
The comments for private variables in
Bootstrap take too much space yet tell very little to unfamiliar reader. Maybe give more descriptive names to your variables and drop comments or replace them by more details. Also one line per comment. Quoting Uncle Bob's fantastic book:Explain yourself in code
-
__construct is generally recommended to be small, yours seems to be doing too much. -
It is generally considered bad practice to have
new Object inside your constructor. This makes it very hard to test and makes your code tightly coupled with dependencies. A better way is to make them explicit dependencies (Dependency Injection):function __construct(Config $CONFIG, SessionManager $SManager, Template $Template)-
Your function
loadController is huge! Quoting Uncle Bob:The first rule of functions is that they should be small. The second rule is that they should be smaller than that.
-
I can see many hard-coded strings inside your functions, you may want to put them separately in a dedicated config class.
-
Writing style: You are using
$app_controller and $modelName. Maybe keep the same convention - either always underscore of camel-case.-
It is a matter of personal preference, but I like to start private/protected function with underscore, to easily distinguish them from public ones.
Code Snippets
function __construct(Config $CONFIG, SessionManager $SManager, Template $Template)Context
StackExchange Code Review Q#30304, answer score: 7
Revisions (0)
No revisions yet.