Take the 2-minute tour ×
Programmers Stack Exchange is a question and answer site for professional programmers interested in conceptual questions about software development. It's 100% free, no registration required.

I've been doing MVC for a few months now using the CodeIgniter framework in PHP but I still don't know if I'm really doing things right.

What I currently do is:

Model - this is where I put database queries (select, insert, update, delete). Here's a sample from one of the models that I have:

function register_user($user_login, $user_profile, $department, $role) {

    $department_id = $this->get_department_id($department);
    $role_id = $this->get_role_id($role);

    array_push($user_login, $department_id, $role_id);

    $this->db->query("INSERT INTO tbl_users SET username=?, hashed_password=?, salt=?, department_id=?, role_id=?", $user_login);
    $user_id = $this->db->insert_id();

        array_push($user_profile, $user_id);

    $this->db->query("
            INSERT INTO tbl_userprofile SET firstname=?, 
            midname=?, lastname=?, user_id=?
        ", $user_profile);
}

Controller - talks to the model, calls up the methods in the model which queries the database, supplies the data which the views will display(success alerts, error alerts, data from database), inherits a parent controller which checks if user is logged in. Here's a sample:

function create_user(){
    $this->load->helper('encryption/Bcrypt');
    $bcrypt = new Bcrypt(15);

     $user_data = array(
        'username' => 'Username', 'firstname' => 'Firstname',
        'middlename' => 'Middlename', 'lastname' => 'Lastname',
            'password' => 'Password', 'department' => 'Department', 
            'role' => 'Role'
    );

    foreach ($user_data as $key => $value) {
        $this->form_validation->set_rules($key, $value, 'required|trim');
    }

    if ($this->form_validation->run() == FALSE) {
        $departments = $this->user_model->list_departments();
        $it_roles = $this->user_model->list_roles(1);
        $tc_roles = $this->user_model->list_roles(2);
        $assessor_roles = $this->user_model->list_roles(3);

        $data['data'] = array('departments' => $departments, 'it_roles' => $it_roles, 'tc_roles' => $tc_roles, 'assessor_roles' => $assessor_roles);

        $data['content'] = 'admin/create_user';

            parent::error_alert();
            $this->load->view($this->_at, $data);

        } else {
        $username = $this->input->post('username');
        $salt = $bcrypt->getSalt();
        $hashed_password = $bcrypt->hash($this->input->post('password'), $salt);
        $fname = $this->input->post('firstname');
        $mname = $this->input->post('middlename');
        $lname = $this->input->post('lastname');
        $department = $this->input->post('department');
        $role = $this->input->post('role');

            $user_login     = array($username, $hashed_password, $salt);
            $user_profile   = array($fname, $mname, $lname);
        $this->user_model->register_user($user_login, $user_profile, $department, $role);

        $data['content'] = 'admin/view_user';

            parent::success_alert(4, 'User Sucessfully Registered!', 'You may now login using your account');


        $data['data'] = array('username' => $username, 'fname' => $fname, 'mname' => $mname, 'lname' => $lname, 'department' => $department, 'role' => $role);
        $this->load->view($this->_at, $data);
    }

}

Views - this is where I put html, css, and JavaScript code (form validation code for the current form, looping through the data supplied by controller, a few if statements to hide and show things depending on the data supplied by the controller).

<!--User registration form-->
<form class="well min-form" method="post">
    <div class="form-heading">
        <h3>User Registration</h3>
    </div>

    <label for="username">Username</label>
    <input type="text" id="username" name="username" class="span3" autofocus>

    <label for="password">Password</label>
    <input type="password" id="password" name="password" class="span3">

    <label for="firstname">First name</label>
    <input type="text" id="firstname" name="firstname" class="span3">

    <label for="middlename">Middle name</label>
    <input type="text" id="middlename" name="middlename" class="span3">

    <label for="lastname">Last name</label>
    <input type="text" id="lastname" name="lastname" class="span3">

    <label for="department">Department</label>
    <input type="text" id="department" name="department" class="span3" list="list_departments">

    <datalist id="list_departments">
        <?php foreach ($data['departments'] as $row) { ?>
            <option data-id="<?php echo $row['department_id']; ?>" value="<?php echo $row['department']; ?>"><?php echo $row['department']; ?></option>
        <?php } ?>
    </datalist>

    <label for="role">Role</label>
    <input type="text" id="role" name="role" class="span3" list="">

    <datalist id="list_it">
        <?php foreach ($data['it_roles'] as $row) { ?>
            <option data-id="<?php echo $row['role_id']; ?>" value="<?php echo $row['role']; ?>"><?php echo $row['role']; ?></option>
        <?php } ?>
    </datalist>

    <datalist id="list_collection">
        <?php foreach ($data['tc_roles'] as $row) { ?>
            <option data-id="<?php echo $row['role_id']; ?>" value="<?php echo $row['role']; ?>"><?php echo $row['role']; ?></option>
        <?php } ?>  
    </datalist>

    <datalist id="list_assessor">
        <?php foreach ($data['assessor_roles'] as $row) { ?>
            <option data-id="<?php echo $row['role_id']; ?>" value="<?php echo $row['role']; ?>"><?php echo $row['role']; ?></option>
        <?php } ?>  
    </datalist>

    <p>
        <button type="submit" class="btn btn-success">Create User</button>
    </p>
</form>

<script>
    var departments = [];
    var roles = [];

    $('#list_departments option').each(function(i){
        departments[i] = $(this).val();
    });

    $('#list_it option').each(function(i){
        roles[roles.length + 1] = $(this).val();
    });

    $('#list_collection option').each(function(i){
        roles[roles.length + 1] = $(this).val();
    });

    $('#list_assessor option').each(function(i){
        roles[roles.length + 1] = $(this).val();
    });

    $('#department').blur(function(){

        var department = $.trim($(this).val()); 
        $('#role').attr('list', 'list_' + department);
    });

    var password = new LiveValidation('password');
    password.add(Validate.Presence);
    password.add(Validate.Length, {minimum: 10});
    $('input[type=text]').each(function(i){
        var field_id = $(this).attr('id');
        var field = new LiveValidation(field_id);
        field.add(Validate.Presence);
        if(field_id == 'department'){
            field.add(Validate.Inclusion, {within : departments});
        }
        else if(field_id == 'role'){
            field.add(Validate.Inclusion, {within : roles})
        }
    });
</script>

The codes above are actually code from the application that I'm currently working on. I'm working on it alone so I don't really have someone to review my code for me and point out the wrong things in it so I'm posting it here in hopes that someone could point out the wrong things that I've done in here. I'm also looking for some guidelines in writing MVC code like what are the things that should be and shouldn't be included in views, models and controllers. How else can I improve the current code that I have right now. I've written some really terrible code before(duplication of logic, etc.) that's why I want to improve my code so that I can easily maintain it in the future. Thanks!

share|improve this question

2 Answers 2

up vote 5 down vote accepted

There are some series of Q&A on how to use mvc pattern in php. Here is a nice answer on MVC details for PHP application - How should a model be structured in MVC?

Another list of posts that might be helpful too:

share|improve this answer

I'm a bit of a purist, I think. To me, models should contain all the logic required to interact with the data model, while the controller should contain the business logic, manipulate the data, and then pass the data to the views. Views should not contain any real logic - although I think display logic (e.g. should this user see this button with label A or label B?) is ok.

So far, the best example I have found of this is in the Yii Framework. Joomla, while purporting to use MVC, generally has very thin controllers and fat models - too much of the actual processing happens in the model when it should be in the controller.

In the end, the logic has to go where it fits best. I do occasionally find myself putting logic that is not strictly data interaction into the model, but usually this is a static function that I need to call from several other controllers. In this case, I feel the DRY principle outranks the pure MVC structure. Sometimes there is this balancing act, and ultimately it's the programmer's call on what goes where.

share|improve this answer

Your Answer

 
discard

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

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