9

I have been wondering what is the right way to write code in OO style in model. Certainly you can have a function that retrieve data from DB and then map to model-level variables. This approach, however, becomes counterintuitive when you have other function in model trying to get other data from BD. For example:

class User extends CI_Model {
    $id, $name, $age .... ;

    public function get_user_from_db_with_id($id) {
        ...
        // get data and map to variable. 
    } 

    public function get_all_users() {
    // return all users in db
    }

}

somewhere in controller:

$user = new User();
$ben = $user->get_user_from_db_with_id($id);

// this does not make sense!! 
$all_user = $ben->get_all_users();

Any thought or comment?

Thanks in advance!

Troy
  • 95
  • 1
  • 1
  • 4

3 Answers3

4

I had to make a similar decision and opted for this (trimmed for clarity)

class UserModel extends MY_Model 
{

    public $UserID = 0;
    public $CustomerID = null;
    public $FirstName = '';
    public $LastName = '';
    public $EmailAddress = '';
    public $Password = null;
    public $UserGroupID = true;     

    function __construct()
    {
        parent::__construct();
    }

    private function get($id)
    {
        $row = $this->get($id);
        if ($row !== null)
        {
            $this->dbResultToObject($row, $this);
        }
    }

    // Return an array of User objects
    public function get_list($deleted = false, $userIDToInclude = null) 
    {
        $params = array(null, $deleted, $userIDToInclude);
        $query = $this->db->call("spUserGet", $params);

        $users = array();
        foreach ($query->result() as $row)
        {
            $user = new UserModel();
            $this->dbResultToObject($row, $user);
            $users[] = $user; 
        }

        return $users;        
    }

    // Other Methods (Validate, Save etc)
}

I use a mixture of public, protected and private properties so that the reflection code I've written to map the properties from the DB results and to the DB sproc calls then only includes the public properties and prevents too many parameters being sent. But that's getting off-topic so my controller then just looks like:

class Users extends MY_Controller 
{

    public function __construct()
    {
        parent::__construct();
        $this->load->model('UserModel', 'user');
    }

    ....

}

Then a list can be retrieved with

$users = $this->user->get_list();

And a single record with

$user = $this->user->get($userID);
Dazz Knowles
  • 534
  • 2
  • 16
  • Hey great. Have you updated your model code? than update here. I have done the same as you. but want to know more functionality if you have updaed – Yatin Mistry Dec 09 '14 at 04:43
1

i'm a big fan of thinking about the design in terms of "roles" - not resources. so a "User" is going to be able to get their own Profile. but its only going to be an "Admin" who is able to get All User Profiles.

so that distinction and important separation between what a User can do - get one record - and what an Admin can do - get all records - starts by having separate controllers for each. The User Controller methods are based upon verifying a single user and granting them access to one record. The Admin Controller methods are based upon verifying an admin and granting them access to all records. And this makes sense even from a URL design standpoint - you want your admin area clearly separate.

This separation at the controller makes everything simpler and easier. When you are resource oriented you are constantly checking the credentials in every method and even in your views. Views should be as simple as possible and not be tasked with "is this person an admin"? When you are role oriented - you check the credentials in the controller - and then your methods, the models, and the views are appropriate for what that 'role' needs to accomplish.

cartalot
  • 3,147
  • 1
  • 16
  • 14
  • I can see where your "roles" approach could save a lot of tedium (constantly checking credentials). I'm going to try that in my next project. I was wondering if you utilized a common specialized controller class for both user and admin objects. – user3413621 Jul 02 '19 at 18:57
0

Since you are think about OO programming, i think you need to think: what does this class represent?

  1. each instance means one user?
  2. each instance means one user-data-generator?

if it's the first case, it makes sense this class has attributes like $id, $name, $age .... and the following codes make sense

$user = new User();
$ben = $user->get_user_from_db_with_id($id);

if it's the second case, it shouldn't have the attributes like $id, $name, $age in you sample.

and these codes

$all_user = $ben->get_all_users();

should be replaced with this

$all_user = $user->get_all_users();
尤川豪
  • 459
  • 5
  • 26
  • Hey, Chuanhao, thank you for replying. I have thought of that too. That really implies that for each table in db, you need to have two classes in your code: One for representing the object and the other to retrieve data and map data into the object-representing class, which is kinda messy. – Troy Jan 17 '14 at 04:54
  • Also, you are not always getting every columns of the table from the database, which means that you are not necessarily mapping data into every single data field in the object-representing class. – Troy Jan 17 '14 at 04:57