Review of my PHP framework, vervPHP

Question

My goal with vervPHP was to create a framework I could create applications in very quickly, without any excessive configuration or over the top framework.

Here's a link to the github, to see more: https://github.com/verv/vervPHP

And now, for some code examples to show how awesome (I think) it is:

index.php:

// Start the session:
session_start();
// Include the framework base:
include_once('framework/verv.php');
// Start the application:
$verv = new verv();

framework/verv.php

class verv {
 // Variables:
 public static $db;
 public static $url;
 public static $rq = array();
 public static $lang = array();
 public static $page = array();
 public static $config = array();
 public static $commands = array();
 public static $framework = array();
 /**
 * Constructor. Do not call manually, unless your module extends verv.
 * 
 * @return null
 * @since 1.0
 */
 public function __construct() {
 // Parse the config:
 self::$config = parse_ini_file('config.ini', true);
 // Setup Language:
 self::setLang();
 // Connect to the database if required:
 self::dbConnect();
 // Prepare the request:
 self::setRQ();
 // Load the remaining framework:
 self::loadFramework();
 // Load the appropriate module:
 self::loadModule(self::getCommand(0));
 // Load the template:
 self::loadTemplate();
 // And finally, render the page:
 self::renderPage();
 }
 // More functions follow...

What I'm hoping from a peer review:

• How can I improve my code (generally speaking)?
• Are there any glaring security holes that I should fix?
• I'm wanting to make this open source (for all the help I recieved on SO while making it) so I need to add an open source license to it - one that allows modification (preferrably with attribution) and allows for commercial development.

And yes, I am in the process of improving the actual documentation - it's pretty shocking (particularly the iCommand chaining) at the moment ;)

• hakre: vervPHP - The constructor that does it all (and you're done)

Answer 1

This is procedural code trying to pretend that it is OO. Just stick a namespace around it and write it like the procedural code that it is:

framework/verv.php

namespace verv;
function setLang() { /* Implementation */ } 
function dbConnect() { /* Implementation */ }
function main()
{
 setLang();
 dbConnect();
 // etc
}

Usage

verv\main();

Advantages

• No more static and self littering the code.
• No loss in usual OO benefits like data hiding and encapsulation, while reducing keystrokes required for implementation.

• EvilChookie: I'm not sure why I ignored namespacing, perhaps I figured it was PHP 5.4+. Anyways, you're spot on in all your points - it's not meant to be procedural MVP OOP, just a few things to make it easier to write simple applications.

Answer 2

You are on a long way to success.

Consider autoload function (better from SPL) to avoid includes, it will also force you to establish good directory structure and clear class naming convention plus more security measures.

Why so many statics? Consider Singleton design pattern if you really need it. Btw, your class is rebuild at every page request anyway, right?

Consider Factory design pattern, or better RAII. Why? Look at your addCommand method, for example - no measures of overwriting commands, and no control on the registerd classes in general.

If you promptly name your registered classes "commands", look at Command and Chain of Responsibility patterns. Observer pattern may suite your needs as well.

Do not forget try catch blocks: if you develop a framework, a lot of unknown persons will use it.

• EvilChookie: Thanks for the feedback. I have a few questions of my own. 1. Could you please elaborate on Autoload? I have exactly two static includes (verv.php and verv.chain.php); everything else is included via glob - what do I get moving to autoload? Why is eight static variables so many? Is there a performance hit? To answer your question, yes, verv is created at every page call - however only one module is ever loaded after that (verv::$rq[0]). You suggest that I should consider several design patterns - what benefits do they bring to the party to what I'm trying to achieve here? I see no ... (cont)
• EvilChookie: ... reason to introduce patterns and design styles 'just because that is what the industry says works'. I'm interested in producing a lightweight framework (and if my cachegrinds are anything to go by it is very quick) that allows for the rapid development of applications (which it does). I am open to improving my work (and I will add more catching and handling, thank you for pointing that out) but I need to know how and why. Thanks for the points :)
• Michael: Since you write a framework, it will grow up fast. Sooner or later you will have problems with source code management. The better and clearer for understanding the code is from very beginning, the faster you will progress.
• Michael: You write a framework, it will grow up fast and must be robust. Sooner or later you will have problems with source code management. The better your code is from the very beginning, the faster you will progress. Autoload will allow you to find files using class names, so you can just new MyClass, or you are at risk to face include dependency problems. See spl_autoload_register() The statics are about the style also, plus you must obey some rules and limitations with them (RTFM) Design patterns are common design problems solutions, not code snipplets, you can implement them in your own way.
• EvilChookie: I don't think source code management is a problem. Frameworks are loaded via glob() (with DIR_NOSORT for speed) from the load folder, and any additional code is introduced at the module level. I've also read about the statics, and I'm happy with the use of them - at any point, you can see what vervPHP is doing through the use of verv::capture(verv::$var) which is immensely helpful when creating an application. Did this framework get loaded? What are the current commands? What is the current request? All these answers are made clear by having static variables (which are then accessed ...

Answer 3

From your code example I'd say you create a class with static global state that by itself is the static global context object of your framework.

That sounds like a very poor design decision to me. All you've got are some global variables and a global function (the constructor). For that you do too much IMHO. The class is not necessary because you don't encapsulate anything.

You can further minimize your design by just having one global variable in form of an array or object and the init function.

Answer 4

The following bit seems far from lightweight. You're loading files the user may not even need. Autoloading would be much lighter as it would load only the files associated with the classes a user attempts to instantiate.

/**
 * Loads the remaining framework, such as databases, forms, scaffolds, etc.
 * You can add your own framework(s) by adding them to the load folder.
 * 
 * @return null
 * @since 1.0; 
 */
public static function loadFramework() {
 // Include any interfaces required:
 include_once('verv.chain.php');
 // Then include any other framework items:
 self::$framework = glob('framework/load/*.php', GLOB_NOSORT);
 foreach (self::$framework as $file) {
 include_once($file);
 }
}

This bit alone will bloat your framework to the point of being unusable. I know this because I once worked at a place that did just this and it led to the framework running out of usable memory fairly easily.

• EvilChookie: That's where the framework resides - it loads the framework, and if the user puts any additional framework files in the same folder, they'll be loaded as well.
• Crashspeeder: I understand what your intention is. I'm telling you that your intention is misguided and you should only load what's needed when it's needed.
• EvilChookie: No problems. Just making sure. As per other comments, I am currently writing a refactor to use an autoloading system (as well as namespacing). I will update my question once it's complete.