When approaching object oriented programming, I always try to adhere to a few best practices:
- Do one thing, and do it well
- Don't Repeat Yourself (DRY)
- Favor composition over inheritance
There are many more, but I've found these three practices to be the most useful in creating modular code. That being said, let's explore how your class compares.
Do one thing and do it well
Your User class is doing many things:
- It is the Domain Model representing a User in the database along with some business logic (hashing the password)
- It is getting data from the
$_POST, coupling this class to the HTTP request layer of your application.
- It is
echo-ing a success flag, coupling this call to the standard output in PHP. Furthermore, if something goes wrong on insert, you can't handle that error. You should be throwing exceptions here instead. See Returning status codes from business layer for additional info
- It is inserting data into the database.
- Worse yet, you appear to have a SQL injection vulnerability since you are using double quotes and variable interpretation in PHP (see Little Bobby Tables for a humorous explanation). You want MYSQL Prepared Statements.
Don't Repeat Yourself
While no code is repeated inside this class, because it is doing multiple jobs it becomes difficult to reuse the code, which could lead to repetitive code later on.
Favor Composition over Inheritance
Your class doesn't inherit from anything, which is good. It doesn't really need to inherit from a super class. Composition (e.g. holding references to other objects specializing in a different but related task) won't help you here either because about 4 layers of an application have been mashed together into one class.
Splitting your class up into multiple classes
While this will seem like overkill at first, the following code will lay the ground work for a modular and testable application.
user.php
This is your domain model representing the database data. This will also contain methods that implement business logic, for instance, hashing the password. It is both data and behavior.
<?php
class User
{
private const PASSWORD_BCRYPT = "...";
private $username;
private $email;
private $password;
public User()
{
}
public User($username, $email, $password)
{
$this->set_username($username);
$this->set_email($email);
$this->set_password($password);
}
public function get_username()
{
return $this->username;
}
public function set_username($value)
{
$this->username = $value;
}
public function get_email()
{
return $this->email;
}
public function set_email($value)
{
$this->email = $value;
}
public function get_password()
{
return $this->password;
}
public function set_password($value)
{
$this->password = password_hash($value, User::PASSWORD_BCRYPT);
}
}
users_repository.php
The "Users Repository" is a class whose sole purpose is to perform CRUD operations on the users table (Create, Read, Update and Delete). It implements an interface that you can use later on as a test harness.
<?php
interface IUsersRepository
{
public function add(User $user);
}
class UsersRepository implements IUsersRepository
{
private static $connection_string;
public static function set_connection_string($value)
{
UsersRepository::$connection_string = $value;
}
private $connection;
public UsersRepository()
{
// $connection = new mysqli(...);
}
public function add(User $user)
{
$statement = $this->connection->prepare('INSERT INTO "users" ("id", "username", "email", "password") VALUES (?, ?, ?, ?)');
$statement->bind_param("id", NULL);
$statement->bind_param("username", $user->get_username());
$statement->bind_param("email", $user->get_email());
$statement->bind_param("password", $user->get_password());
if (!$statement->execute())
throw new Exception("Execute failed: ({$stmt->errno}) {$stmt->error}");
}
}
If the INSERT into the database fails, an Exception gets thrown. Failing to save data to the database is a hard stop. You really can't go any further, so an exception is the ideal way to communicate this catastrophic error (yes, failed database operations are catastrophic and your application should blow up in your face).
users_controller.php
This begins the Poor Man's MVC. The UsersController handles the HTTP request.
Notice that it has two constructors. The first one, which takes no arguments, instantiates UsersRepository as the object responsible for database operations. The second constructor accepts any object implementing the IUsersRepository interface, which is the interface implemented by UsersRepository. We'll use this later on as a test harness so we can write unit tests for this class without a database.
<?php
class UsersController
{
private $repository;
public UsersController()
{
$this->repository = new UsersRepository();
}
public UsersController(IUsersRepository $repository)
{
$this->repository = $repository;
}
public function register()
{
return new User();
}
public function register($params)
{
if (isset($params['username'])) {
$user = new User($params['username'], $params['email'], $params['password']);
$this->repository->add($user);
return '/users/index.php';
}
}
}
users/create.php
Lastly, this file, which technically does too much here, is the actual "Create new User" page:
<?php
$controller = new UsersController();
if (isset($_POST))
{
$redirect = $controller->create($_POST);
header($redirect);
}
else
{
$user = $controller->create(); ?>
<!DOCTYPE HTML>
<html>
...
<body>
<form method="post" action="./create.php">
<input name="username" type="text" value="<?= $user->get_username() ?>">
<input name="email" type="text" value="<?= $user->get_username() ?>">
<input name="password" type="password" value="<?= $user->get_username() ?>">
<button type="submit">Submit</button>
</form>
</body>
</html>
<?php } ?>
It instantiates a new UsersController. If the $_POST is set, then it passes that array into the $controller->create method, which performs the actual logic for extracting the user information from the HTTP request and inserting it into the database. Furthermore, it directs the application flow, for instance redirecting the user to another page upon successful insert (which prevents a double-POST of the same information as well).
The UsersController also has a method just called create() which takes no parameters and returns a User object, which gets displayed in the HTML form when the $_POST is not set.
Really, you'd want to take advantage of one of the many fantastic MVC frameworks out there for PHP for the users_controller.php and create.php files, because the MVC application should be responsible for routing an HTTP request to a controller, and then a specific PHP file for the view should provide the web page.
By splitting up your code into layers, you create a modular and testable application:
- User - Business logic
- UsersRepository - Data storage and retrieval
- UsersController - Handle application flow
- create.php - a not-so-perfect view
Testing your code
Notice that the UsersController has a constructor which accepts an instance of IUsersRepository. This is the interface implemented by UsersRepository. This allows you to write test code that uses a mock or stub user repository instead of going out to a database.
First, create the "mock users repository" class, which implements IUsersRepository
<?php
class MockUserRepository implements IUsersRepository
{
public $added_users = array();
public function add(User $user)
{
$this->added_users[] = $user;
}
}
Next, create your unit test. In this case, I'm using PHPUnit:
<?php
class UsersControllerTest extends PHPUnit_Framework_TestCase
{
public function addsANewUser()
{
$repository = new MockUserRepository();
$controller = new UsersController($repository);
$params = array(
'username' => 'testable',
'email' => '[email protected]',
'password' => 'abc123'
);
$redirect = $controller->create($params);
$this->assertFalse(empty($repository->added_users)));
$this->assertEquals('testable', $repository->added_users[0].get_username());
$this->assertEquals('/users/index.php', $redirect);
}
}
Now you can verify the behavior of your application in an automated and repetitive way. Covering your with unit tests allows you to refactor code or change business rules later on with confidence that you won't introduce bugs.
