mvc example: form post

I've tried to improve my code: separate HTML/PHP, much clear, next time change will be easier. After doing research on MVC/OOP, I've made the below code to learn.

I understand this is not the MVC pattern now. Can anybody help me fix it to become actual MVC?

• get the result from db (list all)

• get the result from db (list search)

index.php

require_once 'grant.php';
require_once 'controller.php';
$controller = new controller();
$controller -> temp_index();

controller.php

require_once 'model.php';
class controller{
    public $model;
    public function __construct(){
        $this -> model = new model();
    }

    public function temp_index(){
        require 'temp_index.php';
        if($_REQUEST['submit'] == 'get_all'){
            $result = $this -> model -> get_all();
            require 'get_all.php';
        }
        else if($_REQUEST['submit'] == 'get_search'){
            $result = $this -> model -> get_search();
            require 'get_search.php';
        }
    }
}

model.php // please ignore not use PDO

```
class model{
public function get_all(){
$sql = "select * from tb order by id desc";
$query = mysql_query($sql) or die(mysql_error());
$result = array();
while($list = mysql_fetch_array($query)){
$result[] = $list;
}
return $result;
}
public function get_search(){
$search = mysql_real_escape_string($_POST['search']);
$search = trim($search);
if($search !== ''){
$sql = "select * from tb where ac_email like '%$search%'";
$query = mysql_query($sql) or die(mysql_error());
if(mysql_num_rows($query) > 0){
$result = array();
while($list = mysql_fetch_array($query)){
$result['result'][] = $list;
}
return $result;
}
else{
$result['statu'] = 'can\'t find search';

Object Oriented Programming Improvements

The first thing you can do to improve code like this is to use dependency injection, instead of instantiating objects with new inside of objects.

// Setup your class hierarchies like this.

abstract class Controller
{
    protected $model;

    protected function __construct(Model $model)
    {
        $this->model = $model;
    }
}

class RootController extends Controller // or HomeController extends Controller
{
    public function __construct(Model $model)
    {
        parent::__construct($model);
    }
}

Then you execute:

$controller = new RootController(new RootModel());  // But, read on.

Notice the use of type hinting in the __construct function. Program to an interface, not an implementation.

Use abstract classes (which you cannot make an instance of) to factor out common properties and methods into one place. Extend abstract classes with specific species of classes. This is where the power of polymorphism via the Strategy Pattern comes in to play!

By type hinting with abstract classes, or interface types, you will not have to change your code simply because a different species of child object need to be used at some other point in time.

If you find that the parents of several classes should have the same functionality or constants, factor again and define a PHP Interface and slap it on to a class like this.

class RootController extends Controller implements Printable
{
    public function __construct(Model $model)
    {
        parent::_construct($model);
    }
}

Where in this case, Printable would be a set of virtual functions, and or constants, that form a contract, forcing any class that implements Printable to define certain public functions / hooks so that it can be used as a "Printable" object by unrelated code in other places.

Model-View-Controller Improvements

In terms of MVC, you may want to consider how a Model is going to send data to a View. Using a Controller's method as the working area for passing dat from a Model to a View is common. If a Controller's method can act as a working space for a particular command, then "M" and "V" (Model and View) can use "C's" method as a place to coordinate things.

Given the following, you could use your programming knowledge to devise a solution where a View is passed data from a Model, without injecting an instance of said Model into a View. In other words, you do not want to do this.

abstract class View
{
    protected $model;

    protected function __construct(Model $model)
    {
        $this->model = $model;
    }
}

class NewsletterView extends View
{
    public function __construct(Model $model)
    {
        parent::__constrcut($model);
    }
}

Summarizing, there are two well known options:

-
Use a method of a Controller (i.e. a command). Have the Model pass a data structure (array or iterable object) to the View.

-
Give the View a reference to the Model and delegate the chore of getting data to the Model within the View. However, as noted this is not what you want to do.

Note that option #2 can create strong coupling between a Model and a View, because the View must then use methods off of the Model instance to get at data. If a Model's methods change (such as the names, or method parameters), the View will have to change, accordingly.

What you want to do is have a setup like this.

abstract class Controller
{
    protected $model;
    protected $view;

    protected function __construct(Model $model, View $view)
    {
        $this->model = $model;
        $this->view = $view;
    }
}

class NewsletterController extends Controller // or HomeController extends Controller
{
    public function __construct(Model $model, View $view)
    {
        parent::__construct($model, $view);
    }

    /**
    * A totally made up method.
    */
    public function register(array $suscriberData) 
    {
        $viewData = $this->model->addSubscriber($suscriberData));
        $this->view=>updateRegistrationPage($viewData);
    }
}

Then you execute:

$controller = new NewsletterController(new NewsletterModel(), new NewsletterView());

(Note: A dependency injection container can resolve class dependencies off screen and just issue you the object you request! Very handy, but not mandatory.)

Your URL to such a command would be:

http://yourdomain.com/newsletter/register

Where "newsletter" is your Controller, and "register" is the command.

Typically, the machinery of a framework (Front Controller / Router / Dispatcher), or just good programming generally, handles transforming the path component of a URL (REQUEST_URI) into an instance of a Controller. In that case, because instances are being created dynamically, you will want to use a class autoloader (as in PHP-FIG PSR-4) at minimum. Using an autoloader can eliminate all but one require statement for accessing classes. You will nee

StackExchange Code Review Q#23160, answer score: 4