Code Review Stack Exchange is a question and answer site for peer programmer code reviews. Join them; it only takes a minute:

Sign up
Here's how it works:
  1. Anybody can ask a question
  2. Anybody can answer
  3. The best answers are voted up and rise to the top

I am creating a PHP app in MVC (no framework). I am having a difficult time understanding how the Model and the Controller communicate in an optimal manner.

User.php

<?php
/**
 * Created by PhpStorm.
 * User: antony
 * Date: 7/7/16
 * Time: 1:17 PM
 */
namespace Fab\Models;

use Fab\Database\DB;

class User
{
    protected $myDB;

    protected $username;
    protected $password;
    protected $isAdmin;

    public function __construct($username, $password)
    {
        $this->myDB = new DB();

        $this->setClassVariables($username, $password);
    }

    /**
     * @return mixed
     */
    public function getUsername()
    {
        return $this->username;
    }

    /**
     * @return mixed
     */
    public function getPassword()
    {
        return $this->password;
    }

    /**
     * @return mixed
     */
    public function getIsAdmin()
    {
        return $this->isAdmin;
    }

    public function setClassVariables($username, $password)
    {
        $user = $this->myDB->getUser($username, $password);

        $user = $user[0];

        $this->username = $user['username'];
        $this->password = $user['password'];
        $this->isAdmin = $user['isAdmin'];
    }

    public function isAdmin()
    {
        if ($this->getIsAdmin() === '1') {
            return "";
        } elseif (is_null($this->isAdmin)) {
            return "The credentials you entered are wrong";
        } elseif ($this->isAdmin === '0') {
            return "You are a user but not an admin..";
        } else {
            return "If you forgot your credentials contact support";
        }
    }

    public function isLoggedIn()
    {
        if (isset($_COOKIE['active'])) {
            return true;
        }

        if (isset($_SESSION['user']) && $_SESSION['user'] == $this->getUsername()) {
            return true;
        } else {
            return false;
        }
    }

    public function login()
    {
        //Start $_SESSION
        $status = session_status();
        if ($status == PHP_SESSION_NONE) {
            //There is no active session
            session_start();
        } elseif ($status == PHP_SESSION_DISABLED) {
            //Sessions are not available
        } elseif ($status == PHP_SESSION_ACTIVE) {
            //Destroy current and start new one
            session_destroy();
            session_start();
        }

        //Set $_SESSION variables
        $_SESSION['user'] = $this->getUsername();
        $_SESSION['password'] = $this->getPassword();
        $_SESSION['isAdmin'] = $this->getIsAdmin();

        //Set $_COOKIE
        if (isset($_POST['remember'])) {
            setcookie("active", $_SESSION['user'], time() + (3600 * 24 * 365));
        }
    }

    public function logout()
    {
        $status = session_status();
        if ($status == PHP_SESSION_NONE) {
            //There is no active session
            session_start();
        } elseif ($status == PHP_SESSION_DISABLED) {
            //Sessions are not available
        } elseif ($status == PHP_SESSION_ACTIVE) {
            //Destroy current and start new one
            session_destroy();
            session_start();
        }

        //Unset $_SESSION variables
        unset($_SESSION["user"]);
        unset($_SESSION["password"]);
        unset($_SESSION["isAdmin"]);

        //Unset $_COOKIE variables
        unset($_COOKIE['active']);
        setcookie('active', '', time() - 3600);
    }

}

Controller.php

<?php
/**
 * Created by PhpStorm.
 * User: antony
 * Date: 7/1/16
 * Time: 9:24 PM
 */
namespace Fab\Controllers;

use Fab\Database\DB;
use Fab\Models\User;
use Twig_Environment;
use Twig_Extension_Debug;
use Twig_Loader_Filesystem;
use Fab\Services\UploadImage;

class AdminController extends Controller
{
    protected $user;

    public function __construct($item = null)
    {
        parent::__construct($item = null);

        $loader = new Twig_Loader_Filesystem(__DIR__ . '/../Views/admin');
        $this->twig = new Twig_Environment($loader, array(
            'debug' => true
        ));
        $this->twig->addExtension(new Twig_Extension_Debug());

        if (isset($_SESSION['user']) && isset($_SESSION['password'])) {
            $this->user = new User($_SESSION['user'], $_SESSION['password']);
        }
    }

    public function index()
    {
        if ($this->adminIsLoggedIn())
            echo $this->twig->render('dashboard.twig');
        else
            $this->login();
    }

    public function addItem()
    {
        if ($this->adminIsLoggedIn())
            echo $this->twig->render('addItem.twig');
        else
            echo $this->twig->render('login.twig');
    }

    public function postAddItem()
    {
        $DB = new DB();
        $uploadImageService = new UploadImage();
        $success = false;

        //Try to upload image
        $uploadError = $uploadImageService->uploadImage();
        if (empty($uploadError)) {

            //Add row to db
            $nameOfImage = $_FILES['image']['name'];
            $result = $DB->addItem($_POST, $nameOfImage);

            if (empty($result)) { //successfully added row
                $flashMessage = "Item Succesfully Added";
                $success = true;
            } else { //failed to add row
                $flashMessage = "Error: Could not add item. Please check the values you have given.";
                //Delete uploaded image from server
                unlink("images/$nameOfImage");
            }

        } else { //image failed to upload
            $flashMessage = $uploadError . "\nError: Could not upload image.";
        }

        echo $this->twig->render('addItem.twig', array('flashMessage' => $flashMessage, 'success' => $success));
    }

    public function deleteItem()
    {
        if ($this->adminIsLoggedIn()) {
            $myDB = new DB();
            $items = $myDB->getAllItems();

            echo $this->twig->render('deleteItem.twig', array('items' => $items));
        } else {
            echo $this->twig->render('login.twig');
        }

    }

    public function postDeleteItem()
    {
        $myDB = new DB();

        $result = $myDB->deleteItems($_POST);

        if ($result == 0) {
            $message = "Success! Items Deleted.";
        } elseif ($result == 1) {
            $message = "Failure. You did not select any items!";
        } elseif ($result == 2) {
            $message = "Failure. Something went wrong. Please try again.";
        } elseif ($result == 3) {
            $message = "Failure. Could not remove image. Make sure you selected a valid item.";
        }

        $items = $myDB->getAllItems();

        echo $this->twig->render('deleteItem.twig', array('items' => $items, 'result' => $result, 'message' => $message));
    }

    public function editItem()
    {
        if ($this->adminIsLoggedIn()) {
            $myDB = new DB();
            $items = $myDB->getAllItems();

            echo $this->twig->render('editItem.twig', array('items' => $items));
        } else {
            echo $this->twig->render('login.twig');
        }
    }

    public function postEditItem()
    {
        $myDB = new DB();
        $result = $myDB->editItems($_POST);

        $items = $myDB->getAllItems();

        echo $this->twig->render('editItem.twig', array('items' => $items, 'result' => $result));
    }

    public function contactSupport()
    {
        if ($this->adminIsLoggedIn())
            echo $this->twig->render('contactSupport.twig');
        else
            echo $this->twig->render('login.twig');
    }

    public function login($errorMessage = null)
    {
        if (isset($errorMessage))
            echo $this->twig->render('login.twig');
        else
            echo $this->twig->render('login.twig', array('errorMessage' => $errorMessage));
    }

    public function postLogin()
    {
        $myDB = new DB();

        $user = $myDB->getUser($_POST['username'], $_POST['password']);

        if (empty($user)) {
            $errorMessage = "Wrong Credentials.";
            $this->login($errorMessage);
        } else {
            $this->user = new User($_POST['username'], $_POST['password']); //find the user from db

            $errorMessage = $this->user->isAdmin(); //authenticate user

            if (empty($errorMessage)) { //if authentication successful

                $this->user->login(); //set Cookies and Session

                $this->index(); //show dashboard
            } else {
                $this->login($errorMessage); //redirect to login page
            }
        }
    }

    public function logout()
    {
        if ($this->adminIsLoggedIn()) {
            $this->user->logout();
            $this->login();
        }
    }

    public function adminIsLoggedIn()
    {
        if (isset($this->user) && $this->user->isLoggedIn() && empty($this->user->isAdmin()))
            return true;
        else
            return false;
    }

}

index.php

<?php
require_once __DIR__ . '/../vendor/autoload.php';
require_once __DIR__ . '/../app/setup.php';

use Fab\Controllers;
use Fab\Router;

$router = new Router\Router();

$router->get('/', 'MainController', 'index');
$router->get('/portfolio', 'ItemsController', 'showAllItems');
$router->get('/portfolio/[\w\d]+', 'ItemsController', 'single_item');
$router->get('/about', 'MainController', 'about');
$router->get('/contact', 'MainController', 'contact');
$router->get('/admin/dashboard', 'AdminController', 'index');
$router->get('/admin/dashboard/addItem', 'AdminController', 'addItem');
$router->get('/admin/dashboard/deleteItem', 'AdminController', 'deleteItem');
$router->get('/admin/dashboard/editItem', 'AdminController', 'editItem');
$router->get('/admin/dashboard/contactSupport', 'AdminController', 'contactSupport');
$router->get('/admin/login', 'AdminController', 'login');
$router->get('/admin/logout', 'AdminController', 'logout');

$router->post('/admin/addItem', 'AdminController', 'postAddItem');
$router->post('/admin/deleteItem', 'AdminController', 'postDeleteItem');
$router->post('/admin/editItem', 'AdminController', 'postEditItem');
$router->post('/admin/login', 'AdminController', 'postLogin');


////See inside $router
//echo "<pre>";
//print_r($router);

$router->submit();

Is this model-controller usage appropriate?

For example:

  1. Should I be initializing a User object in the constructor of the controller using the $_SESSION variables?
  2. In the postLogin() in the controller, I call the getUser() function from the DB twice: once to check if the credentials much a user and once to instantiate the User object. Isn't this overuse?
  3. What do I need a User object (the model I mean) for anyway?
share|improve this question

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Browse other questions tagged or ask your own question.