So, I've been working on a very simple MVC framework and I wanted some feedback on how it works. I've worked it slightly differently where the controllers don't need to extend the base Controller
class (I really didn't see the need to).
Firstly, you'll see the main classes Controller.php
, View.php
and index.php
.
Controller.php
class Controller {
public function __construct($module)
{
$class = implode('', array_map('ucwords', explode('-', $module)));
$model = MODELS . $class . '.php';
$controller = CONTROLLERS . $class . '.php';
if (file_exists($controller) && file_exists($model)) {
require_once $controller;
require_once $model;
$ctrl = '\ctrl\\' . $class;
$model = '\model\\' . $class;
require_once VIEWS . DS . 'partials' . DS . 'header.php';
new $ctrl(new $model(), new View());
require_once VIEWS . DS . 'partials' . DS . 'footer.php';
}
}
}
View.php
class View {
public function __construct()
{
$this->view = null;
$this->data = [];
}
public function load($view)
{
if (!file_exists($view))
exit('View 404: ' . $view);
$this->view = $view;
}
public function set($k, $v)
{
$this->data[$k] = $v;
}
public function get($k)
{
return $this->data[$k];
}
public function render()
{
extract($this->data);
ob_start();
require_once $this->view;
echo ob_get_clean();
}
}
index.php
<?php
session_start();
define('DS', DIRECTORY_SEPARATOR);
define('ROOT', __DIR__ . DS);
define('SRC', __DIR__ . DS . 'src' . DS);
define('VIEWS', __DIR__ . DS . 'app' . DS . 'views' . DS);
define('MODELS', __DIR__ . DS . 'app' . DS . 'models' . DS);
define('CONTROLLERS', __DIR__ . DS . 'app' . DS . 'controllers' . DS);
spl_autoload_register(function ($class_name) {
$core = SRC . 'core' . DS . $class_name . '.php';
$app = SRC . 'app' . DS . $class_name . '.php';
if (file_exists($core)) {
require_once $core;
} else if (file_exists($app)) {
require_once $app;
} else {
exit('Class 404: ' . $class_name);
}
});
if (isset($_GET['module'])) {
$module = $_GET['module'];
$ctrl = new Controller($module);
} else {
$ctrl = new Controller('sign-in');
}
Now we have an actual example:
app
models
controllers
views
partials
header.php
footer.php
sign-in.php
sign-in.php (View)
<h1><?= $title ?></h1>
<form method="post">
<input type="text" name="username" />
<input type="password" name="password" />
<input type="submit" name="sign-in" />
</form>
<?php if (isset($errors)) : ?>
<?php foreach ($errors as $error) : ?>
<li style="font-weight: bold; color: red;">
<?= $error ?>
</li>
<?php endforeach ?>
<?php endif ?>
SignIn (Controller)
<?php
namespace ctrl;
class SignIn {
private $tpl = VIEWS . 'sign-in.php';
public function __construct($model, $view)
{
$this->model = $model;
$this->view = $view;
$view->load($this->tpl);
if (isset($_POST['sign-in'])) {
$this->sign_in();
}
$this->render();
}
public function render()
{
$this->view->set('title', 'Sign In');
$this->view->render();
}
public function sign_in()
{
$errors = [];
if (!isset($_POST['username']) || empty(trim($_POST['username']))) {
$errors[] = 'Username is empty';
}
if (!isset($_POST['password']) || empty(trim($_POST['password']))) {
$errors[] = 'Password is empty';
}
if (empty($errors)) {
$this->model->find();
} else {
$this->view->set('errors', $errors);
}
}
}
Model (Model)
<?php
namespace model;
class SignIn {
public function find()
{
echo 'found';
}
}
I understand the error reporting / validation can be improved for example from the exit
to actual logging along with environment management and the code cann be tidied up but I'm mainly looking for feedback on how the base controller and view class interacts with rest of the system.
1 Answer 1
Ill just jump right in;
Composer To The Rescue
So you have attempted autoloading, thats great! But by modern standards you should be using composer to;
- Manage your dependecies
- Manage autoloading your namespaces
Here is a stackoverflow question that shows how to add your own
Namespaces
Modern PHP should conform to standrads such as the PSR's your namespace is simply model
this isn't good enough, take a look at PSR4, your namespaces should be something like Script47\MVC_FRAMEWORK_NAME\Models
Dependency Injection - The king of kings
There are many design paterns you can use to implement applications, and you've gone for dependency injection, which is great!
But your not doing it quite right, you should use a depedency injection container.
Take a look at php-di (Its a personal faviourte, but there are others to choose from)
This will also do away with your spl_autoload_register
functions!
Controller is a Router not a Controller
Your controller class is resonsible for finding dependcies setting them up and letting them run!
This is not a controller (in my understanding) its a router and;
- It should be named as such (maybe
ControllerRouter
) - It assumes there will be a Model with the same name as the Controller (bad)
- It manually requires certian header files (bad)
- It should use dependecy injection and not hand woven classes / models
Your controller router is going to be one of the more diffuclt classes to write, but it should be a lot of fun.
Web applications use ajax (or node but don't open that box, yet!)
You can POST
to every page to get things to update, but thats annoying for the user (effectivly a page refresh everytime) and doesn't seperate concerns!
You should make ajax requests (a HTTP request) to your "api" which your index & your new ControllerRouter
should be handling and returning prefferably JSON to the client.
Conculsion
There is far to much out of date code for me to sit here and try and improve when the concepts are flawerd. I would recomend you read up on modern PHP techniques and try applying some of them.
Symfony has series on creating your own framework, I have never used it and its ideas and opions may conflict with my own, but you should follow their guide more than my advice