1
\$\begingroup\$

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.

\$\endgroup\$

1 Answer 1

4
\$\begingroup\$

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;

  1. Manage your dependecies
  2. 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;

  1. It should be named as such (maybe ControllerRouter)
  2. It assumes there will be a Model with the same name as the Controller (bad)
  3. It manually requires certian header files (bad)
  4. 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

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.