views:

44

answers:

2

Greetings all, I'm looking for a more efficient way to grab multiple users without too much code redundancy.

I have a Users class, (here's a highly condensed representation)

    Class Users {

    function __construct() { ... ... }

    public static function getNew($id) {

         // This is all only example code
         $sql = "SELECT Name,Email,Etc.. FROM Users WHERE User_ID=?";

         // Retrieval using prepared statements would go here
         $user = new User();
         $user->setID($id);
         ....

         return $user;
}
  public static function getNewestUsers($count) {
     $sql = "SELECT User_ID,Name,Email,Etc.. FROM Users ORDER BY Join_Date LIMIT ?";

     $users = array();

     // Prepared Statement Data Here

     $stmt->bind_result($id,$name,$email,$etc);

     while($stmt->fetch()) {
        $users[] = Users::getNew($id);
        ...
     }
     return $users;
   }
   // These have more sanity/security checks, demonstration only.
   function setID($id) { $this->id = $id; }
   function setName($name) { $this->name = $name; }
 ...
}

So clearly calling getNew() in a loop is inefficient, because it makes $count number of calls to the database each time. I would much rather select all users in a single SQL statement.

The problem is that I have probably a dozen or so methods for getting arrays of users, so all of these would now need to know the names of the database columns which is repeating a lot of structure that could always change later.

What are some other ways that I could approach this problem? My goals include efficiency, flexibility, scalability, and good design practice.

+2  A: 

You could make your User object __construct method accept an array as parameter :

class User 
{

    public function __construct($infos) {
        if(is_array($infos)) {
            $this->_initFromArray($infos);
        } else if(is_numeric($infos)) {
            $this->_initFromID($infos);
        } else {
            //Handle error
        }

    }

    private function _initFromArray($userInfo) {
        //Set your stuff in here
    }

    private function _initFromID($userID) {
        //Do your SQL magic
    }

    public function getUsersNewest($count) {
        sql = "SELECT * FROM Users ORDER BY Join_Date LIMIT ?";

        $users = array();

        // Prepared Statement Data Here
        while($row = mysql_fetch_assoc($res)) {
            $users[] = new Users($row);

        }
        return $users;
    }

}

Something along the line...

Mikushi
Thank you for your help, I will implement your suggestion
pws5068
+1  A: 

Simplification of boilerplate SQL binding

This isn't so much a solution as a suggestion:

You're running pretty bare to the metal here as far as sql. I recommend separating your sql out a bit more, one way that I've found useful is creating a simple database abstraction library function, let's call it query. Using Mikushi's example above:

// params: $select = full select statement with binding placeholders
// params: $bindings = either associative or numeric array, whichever fits your database binding method better
function query($select, $bindings=array()){
$res = array();

// Loop over each binding here
// Run the bound select statement, get the $res result

while($row = mysql_fetch_assoc($res)) {
        $rows[] = $row;

    }
    return $rows;
}

Ideal usage, going off of how binding works with PDO because I'm not really familiar with the binding method for mysql_fetch_assoc()

$user_data = query('SELECT User_ID,Name,Email,Etc.. FROM Users ORDER BY Join_Date LIMIT :count', array('count'=>$count));

That has three benefits: simpler binding, not having to do boiler-plate for single or multi-row select statements (an 80%-of-the-time use case), and potentially an easier time if you ever need to switch databases.

Factory Pattern to Separate Creation of User objects from modification

As far as the original issue of user creation, how about using the factory pattern to separate the creation of User objects from the methods that User objects can perform on their own data?

Class UserFactory{
    public static function getNewestUsers($count){
        $sql_statement = "SELECT User_ID,Name,Email,Etc.. FROM Users ORDER BY Join_Date LIMIT :count";
        $users_info = query($sql_statement, array('count'=>$count);
        $users = array();
        foreach($users as $user_info){
            $users[] = new User($user_info);
        }
        return $users;
    }
}

Use:

$user_objects = UserFactory::getNewestUsers( 45 );

The user object would then only: use __constructor() on an array of data as Mikushi mentions, and perform alterations on that data (which the factory could potentially have a function saveUser($user_object) to save changes.

Tchalvak
I would simply use the full select statement, not -just- a where statement. That allows the `query` function (I'm suggesting it as an actual library function for reusability, not an object method) to be reusable in a lot of places. And the binding could be numeric or, for better clarity, it could be string based, e.g. `$bindings['count'] = 45; $bindings['offset'] = 7; $bindings['username'] = 'bob';`
Tchalvak
Excellent! This will make my code Much more flexible and portable. Your help is greatly appreciated.
pws5068
Whoops, I clarified the UserFactory class above and added a use case because it's better to make the UserFactory methods static so that you don't have to needlessly instantiate the object.
Tchalvak