Small PHP framework template library

Question

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 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:

<?php
/**
 * The main library for the framework
 */
namespace SmallFry\lib; 
class_alias('SmallFry\lib\MySQL_Interface', 'SmallFry\lib\MySQL'); // MySQL class to be deprecated
/**
 * Description of Bootstrap
 *
 * @author nlubin
 */
Class Bootstrap {
 /**
 *
 * @var SessionManager
 */
 private $_session;
 /**
 *
 * @var stdClass
 */
 private $_path;
 /**
 *
 * @var AppController
 */
 private $_controller;
 /**
 *
 * @var Template
 */
 private $_template;
 /**
 *
 * @var Config
 */
 private $CONFIG;
 /**
 * @param Config $CONFIG 
 */
 function __construct(Config $CONFIG) {
 try {
 $this->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($_SERVER["REQUEST_URI"],'?');;
 $request_uri = str_replace("/" . $index , "/", $request_uri);
 $path = $request_uri ?: '/'.$default_controller;
 $path = str_replace("//", "/", $path);
 $path_info = explode("/",$path);
 $page = isset($path_info[2]) ? $path_info[2] : "index";
 list($page, $temp) = explode('.', $page) + array("index", null);
 $model = $path_info[1] ?: $default_controller;
 $obj = (object) array(
 'path_info' => $path_info,
 'page' => $page,
 'args' => array_slice($path_info, 3),
 'route_args' => array_slice($path_info, 2), //if is a route, ignore page
 'model' => $model
 );
 return $obj;
 }
 /**
 * @return AppController
 */
 private function loadController(){
 //LOAD CONTROLLER
 $modFolders = array('images', 'js', 'css');
 //load controller
 if(strlen($this->_path->model) == 0) $this->_path->model = $this->CONFIG->get('DEFAULT_CONTROLLER');
 //find if is in modFolders:
 $folderIntersect = array_intersect($this->_path->path_info, $modFolders);
 if(count($folderIntersect) == 0){ //load it only if it is not in one of those folders
 $controllerName = "{$this->_path->model}Controller";
 $model = $this->createModel($this->_path->model);
 $self = $this;
 $callback = function($modelName) use (&$model, &$self){
 if(get_class($model) === "SmallFry\lib\AppModel") { //check if model DNE
 $newModel = $self->createModel($modelName);
 return $newModel;
 }
 else { //return the original model
 return $model;
 }
 };
 $app_controller = $this->createController($controllerName, $model, $callback); 
 if(!$app_controller) {
 $route = $this->CONFIG->getRoute($this->_path->model);
 if($route) { //try to find route
 $this->_path->page = $route->page;
 $this->_path->args = count($route->args) ? $route->args : $this->_path->route_args;
 $model = $this->createModel($route->model);
 $app_controller = $this->createController($route->controller, $model);
 if(!$app_controller) {
 //show nothing 
 throw new \Exception("You cannot create a controller here for this route ({$route->controller}).");
 }
 }
 else {
 //show nothing 
 throw new \Exception("That controller does not exist ({$controllerName}).");
 exit;
 }
 }
 return $app_controller;
 }
 else { //fake mod-rewrite
 $this->rewrite($this->_path->path_info, $folderIntersect);
 }
 //END LOAD CONTROLLER
 }
 public function createModel($model) {
 $useOldVersion = !$this->CONFIG->get('DB_NEW');
 $mySQLClass = "SmallFry\lib\MySQL_PDO";
 if($useOldVersion) {
 $mySQLClass = "SmallFry\lib\MySQL_Improved";
 }
 //DB CONN
 $firstHandle = null;
 $secondHandle = null;
 //Primary db connection
 $database_info = $this->CONFIG->get('DB_INFO');
 if($database_info) {
 $firstHandle = new $mySQLClass($database_info['host'], $database_info['login'], 
 $database_info['password'], $database_info['database'], $this->CONFIG->get('DEBUG_QUERIES'));
 }
 else {
 exit("DO NOT HAVE DB INFO SET");
 }
 //Secondary db connection
 $database_info = $this->CONFIG->get('SECONDARY_DB_INFO');
 if($database_info) {
 $secondHandle = new $mySQLClass($database_info['host'], $database_info['login'], 
 $database_info['password'], $database_info['database'], $this->CONFIG->get('DEBUG_QUERIES'));
 }
 //END DB CONN
 return AppModelFactory::buildModel($model, $this->CONFIG, $firstHandle, $secondHandle);
 }
 /**
 * @param string $controllerName
 * @return AppController 
 */
 private function createController($controllerName, $model, $callback = null) {
 $nameSpacedController = "SmallFry\\Controller\\$controllerName";
 if (class_exists($nameSpacedController) && is_subclass_of($nameSpacedController, __NAMESPACE__.'\AppController')) {
 $app_controller = new $nameSpacedController($this->_session, $this->CONFIG, $model, $callback);
 } else {
 return false;
 }
 return $app_controller;
 }
 /**
 * @param array $path_info
 * @param array $folderIntersect 
 */
 private function rewrite(array $path_info, array $folderIntersect){
 $find_path = array_keys($folderIntersect);
 $find_length = count($find_path) - 1;
 $file_name = implode(DIRECTORY_SEPARATOR,array_slice($path_info, $find_path[$find_length]));
 $file = \SmallFry\Config\DOCROOT."webroot".DIRECTORY_SEPARATOR.$file_name;
 $file_extension = pathinfo($file, PATHINFO_EXTENSION);
 if(is_file($file)){ //if the file is a real file
 if($file_extension === 'php') {
 include("./webroot/{$file_name}"); exit;
 }
 include \SmallFry\Config\BASEROOT.DIRECTORY_SEPARATOR.'functions'.DIRECTORY_SEPARATOR.'apache_request_headers.php'; // needed for setups without `apache_request_headers`
 // Getting headers sent by the client.
 $headers = apache_request_headers();
 // Checking if the client is validating his cache and if it is current.
 if (isset($headers['IF-MODIFIED-SINCE']) && (strtotime($headers['IF-MODIFIED-SINCE']) == filemtime($file))) {
 // Client's cache IS current, so we just respond '304 Not Modified'.
 header('Last-Modified: '.gmdate('D, d M Y H:i:s', filemtime($file)).' GMT', true, 304);
 header('Expires: '.gmdate('D, d M Y H:i:s \G\M\T', time() + 3600));
 header("Cache-Control: max-age=2592000");
 } else {
 // File not cached or cache outdated, we respond '200 OK' and output the file.
 header('Last-Modified: '.gmdate('D, d M Y H:i:s', filemtime($file)).' GMT', true, 200);
 header('Expires: '.gmdate('D, d M Y H:i:s \G\M\T', time() + 3600));
 header("Cache-Control: max-age=2592000");
 header('Content-Length: '.filesize($file));
 include \SmallFry\Config\BASEROOT.DIRECTORY_SEPARATOR.'functions'.DIRECTORY_SEPARATOR.'mime_type.php'; // needed for setups without `mime_content_type`
 header('Content-type: ' . mime_content_type($file));
 readfile($file);
 }
 }
 else {
 throw new \Exception("File does not exist ({$file}).");
 }
 exit;
 }
}

AppController.php:

<?php
namespace SmallFry\lib;
/**
 * Description of AppController
 *
 * @author nlubin
 */
class AppController {
 private $pageOn;
 protected $name = __CLASS__;
 protected $helpers = array();
 protected $validate = array();
 protected $posts = array();
 protected $session;
 protected $validator;
 protected $template;
 protected $CONFIG;
 /**
 *
 * @param SessionManager $SESSION
 * @param Config $CONFIG
 * @param MySQL_Interface $firstHandle
 * @param MySQL_Interface $secondHandle 
 */
 public function __construct(SessionManager $SESSION, Config $CONFIG, AppModel $model, $modelCallback = null) {
 $this->CONFIG = $CONFIG;
 $this->session = $SESSION;
 if(isset($this->modelName)) { //if the model is different than the controller name
 if($modelCallback !== null && is_callable($modelCallback)) {
 $model = $modelCallback($this->modelName); //get real model
 }
 }
 $modelName = $model->getModelName();
 $this->$modelName = $model;
 /* Get all posts */
 $this->posts = $model->getPosts();
 $view = isset($this->viewName) ? $this->viewName : $modelName;
 $this->CONFIG->set('view', strtolower($view));
 $this->setHelpers();
 if(!$this->session->get(strtolower($modelName))){
 $this->session->set(strtolower($modelName), array());
 }
 }
 private function getPublicMethods(){
 $methods = array();
 $r = new \ReflectionObject($this);
 $r_methods = $r->getMethods(\ReflectionMethod::IS_PUBLIC);
 $notAllowedMethods = array("__construct", "init", "__destruct"); //list of methods that CANNOT be a view and are `keywords`
 foreach($r_methods as $method){
 if($method->class !== 'SmallFry\lib\AppController' && !in_array($method->name, $notAllowedMethods)){ 
 //get only public methods from extended class
 $methods[] = $method->name;
 }
 }
 return $methods;
 }
 /**
 *
 * @param Template $TEMPLATE 
 */
 public function setTemplate(Template $TEMPLATE){
 $this->template = $TEMPLATE;
 $helpers = array();
 foreach($this->helpers as $helper){
 $help = "{$helper}Helper";
 if(isset($this->$help)) {
 $helpers[$helper] = $this->$help;
 }
 }
 $this->template->set('helpers', (object) $helpers);
 }
 /**
 * Function to run before the constructor's view function
 */
 public function init(){} //function to run right after constructor
 public function setPage($pageName) {
 $this->page = $pageName;
 }
 /**
 * Show the current page in the browser
 *
 * @param array $args
 * @return string 
 */
 public function displayPage($args) {
 $this->CONFIG->set('method', $this->page);
 $public_methods = $this->getPublicMethods();
 if(in_array($this->page, $public_methods)) {
 call_user_func_array(array($this, $this->page), $args);
 }
 else {
 throw new \Exception("{$this->name}/{$this->page} does not exist.");
 exit;
 }
 }
 /**
 *
 * @return string 
 */
 function index() {}
 /**
 *
 * @param string $msg
 * @return string 
 */
 protected function getErrorPage($msg = null) {
 $err = '<div class="error errors">%s</div>';
 return sprintf($err, $msg);
 }
 protected function setHelpers(){
 foreach($this->helpers as $helper){
 $help = "{$helper}Helper";
 $nameSpacedHelper = "SmallFry\\helper\\$help";
 if(class_exists($nameSpacedHelper) && is_subclass_of($nameSpacedHelper, 'SmallFry\\helper\\Helper')){
 $this->$help = new $nameSpacedHelper();
 }
 }
 }
 /**
 *
 * @param array $validate
 * @param array $values
 * @param boolean $exit
 * @return boolean 
 */
 protected function validateForm($validate = null, $values = null, $exit = true){
 $this->validator = new FormValidator(); //create new validator
 if($validate == null){
 $validate = $this->validate;
 }
 foreach($validate as $field => $rules){
 foreach($rules as $validate=>$message){
 $this->validator->addValidation($field, $validate, $message);
 }
 }
 return $this->doValidate($values, $exit);
 }
 protected function doValidate($values = null, $exit = true){
 if(!(!isset($_POST) || count($_POST) == 0)){
 //some form was submitted
 if(!$this->validator->ValidateForm($values)){
 $error = '';
 $error_hash = $this->validator->GetErrors();
 foreach($error_hash as $inpname => $inp_err)
 {
 $error .= $inp_err.PHP_EOL;
 }
 return $this->makeError($error, $exit); 
 }
 }
 return true;
 }
 protected function makeError($str, $exit = true){
 $return = $this->getErrorPage(nl2br($str));
 if($exit) exit($return);
 return $return;
 }
 protected function killPage(){ //Throw a 404 for the page
 header("HTTP/1.1 404 Not Found");
 exit;
 }
}

Answer 1

You have asked for critiques, so here it goes.

• 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.

• syb0rg: We could really use a PHP guru programmer in our chatroom. +1
• Dmitri Zaitsev: @syb0rg Thanks, I wish I could call myself PHP guru :) Just read a couple of books.
• Mathieu Guindon: @DmitriZaitsev that doesn't make you any less welcome in The 2nd Monitor ;)
• Dmitri Zaitsev: @lol.upvote Any specific problem you need help with over there?
• qwertynl: @DmitriZaitsev (Neal here) The comments in the Bootstrap class were created by my IDE. Also I use Dependency Injection throughout my code, just in that one class I create everything and pass it on to all of the other classes. Also yes I know I am inconsistent in my variable naming (I will try to fix that in the future). Is there anything else that you can add? (I have a feeling you only read the code int he question and not the code in the repository)

Answer 2

Small nitpick,

class_alias('SmallFry\lib\MySQL_Interface', 'SmallFry\lib\MySQL'); // MySQL class to be deprecated

You can use the use statement:

use SmallFry\lib\MySQL_Interface as MySQL

• Chad Retz: class_alias is for more than just the current file
• Naftali: Exactly @ChadRetz I did not want to change my code in older files that used an older version of my lib.