Attempting to utilize OOP with a user-management class

Originally, this isn't how I would have done this at all. I was told by someone I know who is a programmer that I needed to include several functions: one for removing and adding users, one for searching for users information etc.

I would have made it so you can pass a username into the constructor and then get all the user info and leave it to another UserManagement class to take care of things like adding and removing users, but I was told (very very forthrightly) that, that was NOT the way of going about doing what I'm doing.

Am I using OOP correctly? I have asked 2 people and their opinions are almost directly opposite to each others. The code below works fine, performs well and does what I want it to just fine, but I'm worried that I'm not quite getting when and when not to use OOP.

```
class User{
public $username;
public $uid;
public $date;
public $location;

public function __construct(){
}

public function getUser($user){

$getUserInfo = new Database;
$getuserInfo("SELECT * FROM users WHERE username = ?", array($user));
$result = $getUserInfo->result;

foreach ($result as $value) {
$this->username = $value['username'];
$this->uid = $value['uid'];
$this->date = $value['date'];
$this->location = $value['location'];
}
}

public function getUserByUID($uid){
$getUserInfo = $GLOBALS['databaseUserManagement']->prepare("SELECT * FROM users WHERE uid = ?");
$getUserInfo->execute(array($uid));
$result = $getUserInfo->fetchAll();

foreach ($result as $value) {
$this->username = $value['username'];
$this->uid = $value['uid'];
$this->date = $value['date'];
$this->location = $value['location'];
}
}

public function deleteUser($username){
$deleteSpecifiedUser = $GLOBALS["databaseUserManagement"]->prepare("DELETE FROM users WHERE username = ?

No, this design is not object-oriented, nor would I recommend you follow your friend's advice. I also would not recommend your original design either. Neither one adheres very well to object-oriented design principles, namely the SOLID principles.

In your friend's rendition, he is proposing a structure that is very active record-like, where a domain object also knows how to persist and retrieve itself. While this pattern is used in many places (Ruby on Rails, CakePHP, etc), it can make testing very difficult, it can cause dependency problems, and it is not single responsibility.

It is difficult to test (especially your rendition) because the object is dependent on the database. Since you instantiate the database object in your method, there is no easy way to mock it or stub it out either.

Nor is there an easy way to substitute different implementations. Your class is tied to the concrete database implementation. It has a dependency that cannot be isolated.

The class has numerous responsibilities that are completely unrelated to representing a user. For example, in the getUser method, we are responsible for:

• Getting a database connection object,

• Building a query,

• Running that query,

• Fetching the results,

• Translating the results into object properties.

This code has a lot of fingers in a lot of pies, and depends on the specific functionality of a lot of other parts.

Your proposal of simply having a data object, and a separate object to manage persistence is more on track. It falls down a bit in the details, though. To be truly object-oriented, you want to encapsulate your data. Having all public members does not encapsulate anything. The most basic way to make this more object-oriented is to have getters and setters (some may debate this, and there are other routes that can be taken, but this is a good start). This way, if the internal structure of your class changes, you can hide it by adjusting the implementations of the encapsulating methods while preserving the public API.

The pattern you are looking for is the Repository pattern. It allows you to isolate and encapsulate mapping between a persistence layer and domain objects, and allows the domain objects to focus on the single responsibility of representing the business concept they represent.

I would also recommend you learn about dependency injection to help you to understand why instantiating objects inline can cause problems, and what one potential solution is.

StackExchange Code Review Q#57487, answer score: 5