User class design

Explained what I'm doing in the comments:

class User {

    public $id;
    public $email;
    public $username;
    public $password;
    public $rep;

    public function __constructor($id, $email, $username, $password, $rep) { // Assign all 'required' fields for our object upon instantiation

        $this->id = $id;
        $this->email = $email;
        $this->username = $username;
        $this->password = $password;
        $this->rep = $rep;

    }

    public static function getUsers($query, $mysqli) { // Pass the query to select desired users and the mysqli connection object

        $all_users = $mysqli->query($query);
        $user = array();
        while($users = $all_users->fetch_assoc()) {

            $temp_user = new User($users['id'], $users['email'], $users['username'], $users['password'], $users['rep']);
            $user[] = $temp_user;

        }

        return $user;

    }

}

Which allows me to do something like this:

$users = User::getUsers("SELECT * FROM users", $mysqli); // Returns an array of user objects containing each users details

Which I can loop through at will.

Is using a static function like this a good idea? What might be the pitfalls to my design? One that just occurred to me when writing this is that I might not always needs every user field, but it wouldn't be hard to customize the constructor to allow only desired fields to be assigned values.

Here is what I was doing BEFORE the previous example. Is this technically better? The thing that was bothering me is there are several occasions where I'm duplicating the block of code below to instantiate an array of objects. Should I create another class to take care of this process, or am I still going about this the wrong way?

```
$all_users = $mysqli->query($query);
$user = array();
while($users = $all_users->fetch_assoc()) {

$temp_user = new User($users['id'], $users['email'], $users['username'], $users['password'], $users['rep']);
$user[] = $temp

No, this is not OO code.

You are missing some of the key parts of OO.

Data Hiding

All of the class properties are public. Use protected for id, email, username etc. This stops outsiders from modifying the object without your control. The object should only be modified using the public interface that you provide for it.

Encapsulation

Did you implement a public interface for your object, while hiding implementation details away from the programmer who will use your object?

I think the answer is that you haven't really provided a complete interface here. Unfortunately your one method is misleading. Your class is a User class, however it returns Users (plural). This class actually looks like a DataMapper for users rather than a User class. This also leads me to think that you don't need the properties in the object as you will likely just want to be returning the results.

Static

OO and static methods do not mix. You kind of hit it with your usage example. It will work without you having any object created.

$users = User::getUsers("SELECT * FROM users", $mysqli);

Observe how this is really just calling a function. None of the class properties are used within the method.

I don't believe there is any place for static methods in OO. Also, static causes a tight coupling in your code. Look at your usage, what will you need in place to run that code? See if you come up with the same answer as me before you read on.

The things you will need to run the static code are: A class named User with a method named getUsers.

Now, look at the following call to an object method and tell me what you need:

$user->getUsers("SELECT * FROM users", $mysqli);

Answer: An object with a getUsers method. You have just removed your dependency on the User class. Now if you want to you can pass in a SuperUser object and get super users. This is an example of polymorphism which is one of the key concepts of OOP.

Testability

By calling User::getUsers in your code you have a hard-coded break out of the unit, making the code very difficult to test as a unit. Using an object allows you to pass in a special object to help you test.

StackExchange Code Review Q#11004, answer score: 6