SQL query efficiency when using PHP classes

Question

I've been programming in PHP for many years, however only recently started programming with classes. I have the following - basic - user class as follows:

<?php
/* The class for constructing any user's information
 */

    class User {

        protected $userId, $email, $userGroup;

        protected function getEmail() {
            return $this->email;
        }

        protected function getUserId() {
            return $this->userId;
        }

        protected function getUserGroup() {
            return $this->userId;
        }


        public function __construct($userId='') {
            if($userId) {
                $select = mysql_query("SELECT userId, email, user_group FROM user WHERE userId = '$userId'");
                while($user==mysql_fetch_array($select)) {
                $this->email = $user[email];
                    $this->userId = $userId;
                    $this->userGroup = $user[user_group];
                }
            }
        }

}?>

So I understand I can do the following

<?php
$user = new User($userId);
echo $user->getEmail();
?>

To display the user's email address for a given userId. What I'd like to know is, what would be the best way - using OOP - to display, say, 40 user's emails. Obviously creating 40 user objects would be silly as that's 40 SQL queries. Would you simply make a "users" class that was used for returning an array of multiple users, after doing an SQL given various parameters?

Ie

<?php
$getUsers = new Users('Tom');
// create 'Users' object where name is 'Tom'
print_r($users);
// prints the returned array of users?
?>

Thanks for your help. Hopefully this is clear.

Where does $userID normally come from? If it's from an external source, you're prone to SQL injection. Please switch to prepared statements to prevent that. BTW, the mysql_* functions are deprecated. — Marcel Korpel
– Marcel Korpel, Commented Oct 14, 2013 at 10:52
Hi Marcel. I will add security features to my SQL at a later stage - this is just a basic early code whilst I clear up my initial question in my head. Thanks for pointing out about mysql_* - I wasn't aware of this. — Tom
– Tom, Commented Oct 14, 2013 at 10:58
@Tom : have look into PDO. With PDO u can cast a recordset to an array of ex. User. This way u only need 1 SQL and u get 40users in return — DarkBee
– DarkBee, Commented Oct 14, 2013 at 10:59

Adam Rivers · Accepted Answer · 2013-10-14 17:02:50Z

7

I'd do it something like this (using another class):

class UserRepository {
    public function getByName($name) {
        $result = mysql_query("SELECT userId, email, user_group FROM user WHERE name = '$name'");

        $users = [];

        while ($row = mysql_fetch_assoc($result)) {
            $user = new User;
            $user->email     = $row['email'];
            $user->userId    = $row['userId'];
            $user->userGroup = $row['user_group'];

            $users[] = $user;
        }

        return $users;
    }
}

Addition: The following example gives a good idea on how you can make the classes more testable and easy to modify in the future should they need to be:

UserRepositoryInterface

interface UserRepositoryInterface {
    public function getByName($name);
    public function getByUserId($id);
}

MySqliUserRepository

class MySqliUserRepository implements UserRepositoryInterface {
    public function getByName($name) {
        // Get by name using mysqli methods here
    }

    public function getByUserId($id) {
        // Get by user id using mysqli methods here
    }
}

PDOUserRepository

class PDOUserRepository implements UserRepositoryInterface {
    public function getByName($name) {
        // Get by name using PDO methods here
    }

    public function getByUserId($id) {
        // Get by user id using PDO methods here
    }
}

Usage

class Foo {

    protected $userRepository;

    public function __construct(UserRepositoryInterface $userRepository) {
        $this->userRepository = $userRepository;
    }

    public function bar() {
        $user = $this->userRepository->getByUserId(10);
    }
}

Regarding use of mysql_

It may not be exactly how you do it but it'll give you an idea. Also mysql_ is depreciated so its best to use mysqli_ or PDO (my personal recommendation). PDO is also much more OOP friendly.

PDO: http://php.net/manual/en/book.pdo.php

mysqli_: http://php.net/manual/en/book.mysqli.php

Update:

Your individual user class would simply contain information relating to the user. The user class shouldn't contain any way to retrieve a user, that is the job of the repository. So if you want to retrieve 1 user, instead of doing in the User __construct as you currently do, add a method to the UserRepository that looks something like this:

public function getByUserId($id) {
    // Select user from db, check only 1 exists, make user object, return.
}

edited Oct 14, 2013 at 17:02

answered Oct 14, 2013 at 10:59

Adam Rivers

1,0757 silver badges13 bronze badges

Sign up to request clarification or add additional context in comments.

7 Comments

Adam Rivers Over a year ago

@DarkBee I've explained that its depreciated and recommended alternatives, he says he just wants to get an idea on how to do it and used mysql_ in his original question.

Tom Over a year ago

Thank you for your response. Would you still have an individual user class as well though, for individual user specific things? For example, $user->hasAccess($page); or something like that? Or would you simply have a method on this e.g checkUserAccess($userId, $page); ? Thanks for your help

Tom Over a year ago

So to clarify. You would have something like $userRepository = new UserRepository(); $user = $userRepository->getByUserId('41'); echo $user->getEmail(); ? Cheers

Adam Rivers Over a year ago

@TomMac Yes, that's how I would do it, but obviously if the id is an integer it'd look like: $user = $userRepository->getByUserId(41);.

Bill Karwin Over a year ago

@DarkBee, just because ext/mysql is deprecated doesn't mean projects currently using it will run out and rewrite all their code. It just means new projects should avoid using it. It's reasonable for Adam to give an answer using the same API that the OP used. Your downvote is unwarranted.

|

Álvaro González · Accepted Answer · 2013-10-14 11:43:57Z

1

I try to separate my data objects from the DB stuff. In your case, I'd make the following arrangements:

An instance of the User class represents an individual user, either in DB or not. The constructor does not retrieve anything from DB, it just populates class properties.
For users not in DB (e.g., a newly created user) the userId property is NULL (not '').
Methods that do DB stuff expect a database interface (or at least an object) as argument:
```
public function save(PDO $pdo){
}
```
There're static methods to fetch stuff from DB where a class instance does not make sense yet; they return either a User instance or a User collection:
```
public static function fetchById(PDO $pdo, $id){
}

public static function fetchAll(PDO $pdo){
}
```

When it makes sense, I write a private method to share common code:

private static function fetch(PDO $pdo, array $filter=array()){
    $sql = 'SELECT id, email, group
         FROM user' . PHP_EOL;
    $params = array();

   if( isset($filter['id']) ){
       $sql .= 'WHERE id=:id';
       $params['id'] = $filter['id'];
   }

   //...
}

public static function fetchById(PDO $pdo, $id){
    $data = self::fetch($pdo, array('id' => $id));
    if( empty($data) ){
        return NULL;
    }else{
        reset($data);
        return curren($data);
    }
}

public static function fetchAll(PDO $pdo){
    return self::fetch($pdo);
}

The User collection I've mentioned can as simple as an array of User instances, or as elaborate as your own generics implementation.

This way, a typical script looks like this:

// Fetch my details
$me = User::fetchById(1);

// Create a new user
$you = new User(NULL, 'Joe', 'Guests');
$you->save($pdo);
$message = sprintf('User created with ID=%d', $you->userId);

answered Oct 14, 2013 at 11:43

Álvaro González

147k45 gold badges282 silver badges378 bronze badges

4 Comments

Tom Over a year ago

So are your fetch methods being housed under the User class? With this, if you did $me=User::fetchById(1); could you then say echo $me->userId; ? Would you need to define the constructors still? Thanks

Álvaro González Over a year ago

@Tom - #1 and #2: correct. #3: Constructor is not mandatory but lack of one would turn $you = new User(NULL, 'Joe', 'Guests'); into 4 lines of code. Also, please note you'll make heavy use of the constructor within the fetch... methods. My criterion is to define important/mandatory properties in the constructor and assign additional secondary properties somewhere else (e.g., with a setter function) but I guess the important bit is to be coherent.

Tom Over a year ago

I think I've got it. Does this mean if you did $users = User::fetchAll();, would this then mean $users is an array of User objects. ie, echo $user[0]->userId; would echo the first user's userId? Thanks again

Álvaro González Over a year ago

Exactly. Also, note that Adam's UserRepository is a more sophisticated solution to this same problem.

VancleiP · Accepted Answer · 2013-10-14 11:05:49Z

0

Instead of using the construct to retrieve a user, you could use methods to insert/retrieve users instead.

You can create methods to insert new users, update a particular user, retrieve one particular user or retrieve all (with conditions) in an array of users' objects, etc

edited Oct 14, 2013 at 11:05

answered Oct 14, 2013 at 11:00

VancleiP

6574 silver badges7 bronze badges

Collectives™ on Stack Overflow

SQL query efficiency when using PHP classes

3 Answers 3

7 Comments

4 Comments

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

3 Answers 3

7 Comments

4 Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Related