2
\$\begingroup\$

I'm new to the MVC model. This is where I started: User authentication system. After questioning, reading and thinking a lot, I was able to write a small framework (don't know if 'framework' is the right word).

I will try to explain everything as simple as possible. I hope you could tell me what I'm doing wrong and what not.

The site structure (folders are bold):

  • index.php
  • /modules
  • /javascript
  • /images
  • /css
  • /applications
    • config.php
    • Router.class.php
    • Registry.class.php
    • /model
      • /entities
        • User.class.php
        • Connection.class.php
      • /mappers
        • UserMapper.class.php
      • /services
        • AuthService.class.php
        • RegistrerService.class.php
    • /controller
      • HomeController.class.php
      • AuthController.class.php
      • RegisterController.class.php
      • BaseController.class.php (gets extended by all controllers)
    • /view
      • View.class.php
      • /templates

Is this a good site structure? Do you have any tips/suggestions?

Here are some important scripts that show how everything works:

index.php

<?php
require_once('application/config.php');

$registry = new Registry();
$router = new Router($registry);
$view = new View($registry);

$router->setPath('application/controller');

$registry->con = Connection::get();
$registry->router = $router;
$registry->view = $view;

//Load page
$router->load();

?>

<a href="index.php?route=login">Login</a> <a href="index.php?route=register">Register</a>

config.php

error_reporting(E_ALL);
session_start();

require_once('application/Registry.class.php');
require_once('application/Router.class.php');

//Autoload classes
function __autoload($className) {

    if (file_exists($class = 'application/model/entities/' . $className . '.class.php')) { //Entities
        require_once($class);
    }
    else if (file_exists($class = 'application/model/mappers/' . $className . '.class.php')) { //Mappers
        require_once($class);
    }
    else if (file_exists($class = 'application/model/services/' . $className . '.class.php')) { //Services
        require_once($class);
    }
    else if (file_exists($class = 'application/controller/' . $className . '.class.php')) { //Controllers
        require_once($class);
    }
    else if (file_exists($class = 'application/view/' . $className . '.class.php')) {  //View
        require_once($class);
    }

}

Registry.class.php

<?php
class Registry {

    private $vars = array();

    public function __set($index, $value) {
        $this->vars[$index] = $value;
    }

    public function __get($index) {
        return $this->vars[$index];
    }

}

Router.class.php

This script calls the correct controller (and action). index.php?route=auth/index will call the index() function of AuthController.class.

AuthController.class.php

<?php
class AuthController extends BaseController {

    public function index() {
        //Create new authenticationService and pass connection variable
        $authService = new AuthService($this->registry->con);

        if($authService->login('test@test.com', 'testPass')) {
            $notice = 'Test logged In';
        } else {
            $notice = 'Not logged in';
        }

        $this->registry->view->heading = 'Login';
        $this->registry->view->notice = $notice;

        //Show the template 'test.php'
        $this->registry->view->show('login');
    }

}

View.class.php

I will only show the show() function of this class because it is the most important. It loads the correct template.

public function show($templateName) {
    $template = 'application/view/templates/' . $templateName . '.php';

    if (!file_exists($template)) {
        die('Template not found');
    }

    //Set template variables
    foreach ($this->vars as $key => $value) {
        $$key = $value;
    }

    //Include template file
    include($template);
}

login.php

This file is within the templates folder:

<h1><?php echo $heading; ?></h1>
<?php echo $notice; ?>

Am I doing it right (according to the MVC model)?

\$\endgroup\$
3
\$\begingroup\$

Is this a good site structure? Do you have any tips/suggestions?

  • You might wanna create a new folder public / public_html to store all the application's public files for the web (javascripts, stylesheets, images, front-end controller, etc). Then you can set this folder as your web root so that the rest of your application code remains private. It also has the benefit of when the PHP module fails to load, only your front-end controller's code will be exposed. The rest will remain hidden.

  • I don't see a reason why you wouldn't merge config.php with index.php. They essentially do the same thing; bootstrapping the "framework".

  • May I suggest you to make use of a namespace based auto loader (e.g. the PSR-0 autoloader).

    With this you don't have to keep adding new folders to your auto loader (referring to the long chain of else ifs in your current auto loader). Instead, you declare them via namespaces in your classes which makes it more efficient and handy. Additionally it prevents conflict between duplicate class names from different namespaces.

Notes

  • Your controllers are tightly coupled to their dependencies. I suggest you make use of Dependency Injection to couple them loosely. It also makes for easier testing.

  • Your Registry class introduces global state which goes against the principles of OOP and is considered an anti-pattern.


HTML code in the index.php bootstrap file is a big no no.

<a href="index.php?route=login">Login</a> <a href="index.php?route=register">Register</a>

Same goes for:

if($authService->login('test@test.com', 'testPass')) {
    $notice = 'Test logged In'; //<---
} else {
    $notice = 'Not logged in'; //<---
}

In the MVC pattern all View related logic goes in it's corresponding View class and HTML in the template.


In the show() method of your View class you're doing:

foreach ($this->vars as $key => $value) {
    $$key = $value;
}

Fortunately, extract() gets the job done with less code: extract($this->vars);.


Am I doing it right (according to the MVC model)?

The MVC pattern is all about separation of concerns. You've almost got that right. Make sure every type of logic resides in the appropriate layer, and you will be doing it right.

As far as the rest of your code (the "framework"), that's just detail that has nothing to do with the MVC pattern directly. The only thing it serves is to bootstrap itself so that you can utilize the MVC pattern in an easy, efficient manner.

\$\endgroup\$
  • \$\begingroup\$ Thanks again kid! The reaction is a little late becuase I was still figuring everything you said out. I will sure use this information. \$\endgroup\$ – JasonK Sep 9 '14 at 16:58

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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