Is my PHP controller structured the wrong way? Code in the wrong place

Question

I've fairly new to PHP MVC and I'm not 100% sold I'm doing this in the most logical way possible. This is a piece of code from one of my controllers. A friend said a majority of this code should be moved into the Model? Is this so?

Right now I have a DataAccess layer that does all the SQL work. For User work I have a UserDataAccess object, which will create, update and delete. It appears I'm using something along the lines of the Data Mapper pattern. My Model classes map directly to the database columns on a given table and that's it.

I'm continually confused at where each piece of code should go.

// Posted values passed all checks.
        if(count($errors) == 0)
        {                   
            try
            {
                // Get a Connection.
                $connection = ConnectionFactory::GetConnection('mysql');

                try
                {
                    $connection->beginTransaction();                

                    // Create DataAccess object.
                    $uda = new UserDataAccess();

                    // Check if username and email are avail.                       
                    if(count($uda->CheckUsername($connection, $user->Username)) > 0)
                        $errors['username_taken'] = "Username is taken";
                    if(count($uda->CheckEmail($connection, $user->Email)) > 0)
                        $errors['email_taken'] = "Email is taken";      

                    if(count($errors) == 0)
                    {
                        // Perform Create.
                        $userId = $uda->CreateUser($connection, $user->GetArray());
                        // Add User to Rookie Role.             
                        $uda->AddUserToRole($connection, 1, $userId);   
                        // Commit connection.
                        $connection->commit();
                    }

                    // Clear connection.
                    $connection = null;

                    // Redirect user.   
                    if(count($errors) == 0)
                        header('location: /' . $user->Username);
                }
                catch(PDOException $e)
                {
                    $connection->rollBack();
                    $errors['internal'] = "Opps: There was an error processing the request.";
                }
            }
            catch(Exception $e)
            {
                $errors['internal'] = "Opps: There was an error processing the request.";   
            }                               
        }   
    }

Answer 1

From the looks of it, it seems like some code that sound go into the controller.

Here's a break down of MVC:

• Model: Accessing database, preferably using an object oriented method that treats data like object.
• View: The HTML of the page.
• Controller: Logic that allows a dynamic page to be built.

That probably sounded really abstract. The general idea of views is that they are the HTML template, with minimum codes, preferably only echoing certain dynamic element of the page (NOT THE HTML, just plain text, usually) and/or a bit of if and foreach loops. Example:

.... Your HTML code
<?php foreach ($page->users as $user): /* Loops through all the users */ ?>
<li><?php echo $user->name; /* echo their name */ ?></li> /
<?php endforeach; ?>
.... Your HTML Code

The idea behind controllers is how you get your page to actually work, by handling the logic, getting input, and such. Example:

class Controller extends BaseController{
    function indexpage($args=array()){
        if ($args[0] == 'user') $user = UserModel::listUsers(); // If the argument to the controller is 'user' (could be provided by GET, or just the URL, we won't go into that), list all users.
        $this->render('yourViewPage', array('user' => $user)); // renders the view file named yourViewPage.php and pass the value of $user into a variable named 'user'. As shown by above accessed via $page->user.
    }
}

Granted that the above is only a simple example, you get the point. The render() renders the page and passes the key => value pair in the array so that the view has access to them.

The Model is the database interaction. It allows you to access the database without using SQL (preferably). Example:

class UserModel{
    public $name;
    public $openid;
    public static function listUsers($offset=0, $max=20){
        global $persister;
        return $persister->list('UserModel', 0, 20, array('order'=>'NAME DESC'));
    }
}

// Create a new user. This usually goes into the controller

$user = User(); 
$user->name = 'Your User'; // sets name
$user->openid = 'htto://theiropenidprovider.com/openid'; // sets openid
$persister->track($user); // Adds the user to be tracked. (the tracking info is usually written in XML, but we won't go into that).
$persister->flushAll(); // Saves to the database (kinda like commit)
// Gets the user.
$persister->find('UserModel', 'name', 'Your User') // Find the user that has the name of "Your User" in all UserModel instanced tracked.

So Models don't have to have the most coding. In my opinions controllers would have a lot of code, but that totally depends on the complexity of what you're building.

Hope that clears it up for you.

Answer 2

Try to move the mysql connection entirely to the model, the controllers should not know what type of database you are using and how you are connecting to it. Try to think of it in this way:

• Views only show pages to the users;
• Controllers answer to requests by performing calculations, interacting with the Models and loading Views;
• You should always be able to change any of the components without affecting the ones that interact with it;

Answer 3

As you said, the Model is where you should put your dabatase code above. A controller handles what the user sees. In PHP the controller is often not a class but to keep things practical, just the php file which the user navigates to, which controls what happens for that view. The controller should not need to know how the DB is implemented, or even that a DB is used, so for example having transaction information is not good.