HiveBrain v1.2.0
Get Started
← Back to all entries
patternphpMinor

Database abstraction layer for PHP web application

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
layerapplicationphpabstractiondatabaseforweb

Problem

I'm working on a PHP based web application.

While building the UserStorage class, (which acts as a storage source for user data, and implements the UserStorageInterface), I noticed something. The get method fetches the user data from the storage source, and returns it:

public function get($identifier = null, $alias = null, $email = null) {
    //Fetch the data from MySQL (PDO)
}


Here, identifier, alias and email all are unique to each user. The reason I made null their default value, is that the method requires only one of them to execute.

I thought, why not put every needed parameter in a object? Then I can limit/extend the functionality of the method as desired, through the provided parameters. Thus I decided on a class named Schema (which means a plan of action or something like that, I believe). Please correct me if the term I used is the wrong one.

The Schema class:

```
namespace Whelp\Commons\Schema;

use Whelp\Commons\DataTypes\Int;
use Whelp\Commons\DataTypes\String;

class Schema {

private $source = [];
private $params = [];
private $conditions = [];

private $readyForUse = [];

public function __construct(array $source = [], array $params = []) {
$this->source = $source;
$this->params = $params;
$this->validate();
}

/**
* @throws \InvalidArgumentException if a parameter which is not optional, and MUST be provided, is missing
*/

private function validate() {

if(array_key_exists('conditions', $this->source)) {
$this->conditions = $this->source['conditions'];
unset($this->source['conditions']);
}

foreach ($this->source as $param => $prop) {

//If the schema parameter is not optional and the optional key has not been omitted, the provided parameters must contain the schema parameter
if( array_key_exists('optional', $prop) && !$prop['optional'] && !array_key_exists($param, $this->params)) {

Solution

You appear to have three ways to uniquely identify a row in the database representing a user. The UserStorage class appears to be an implementation of the Repository Pattern. The architectural problem here lies with the Schema classes. You are creating several new classes to reduce the numbers of arguments to your UserStorage methods and to hold the data used in a database operation. It would be better to create a Domain Model for a user, and have the UserStorage class just deal with the User Domain Model.

The UserStorageInterface with strongly defined methods for finding users:

interface UserStorageInterface
{
    public function findById($id);
    public function findByAlias($alias);
    public function findByEmail($email);
    public function update($user);
    public function create($user);
    public function remove($user);
}


The UserStorage class implementing the interface:

class UserStorage implements UserStorageInterface
{
    public function findById($id) {
        $data = // ... execute query and find by id

        return $this->mapToUser($data);
    }

    public function findByAlias($alias) {
        $data = // ... execute query and find by alias

        return $this->mapToUser($data);
    }

    public function findByEmail($email) {
        $data = // ... execute query and find by email

        return $this->mapToUser($data);
    }

    private function mapToUser($data) {
        $user = new User($data['id']);
        $user->setAlias($data['alias']);
        $user->setEmail($data['email']);

        return $user;
    }

    public function create($user) {
        // INSERT
        $user->setId(/* new Id */);
    }

    public function update($user) {
        // UPDATE
    }

    public function remove($user) {
        // DELETE
    }
}


Lastly, the User Domain Model:

class User
{
    private $id;
    private $alias;
    private $oldAlias;
    private $oldEmail;
    private $email;

    public __construct($id = null) {
        if (isset($id)) {
            $this->setId($id);
        }
    }

    public function getId() {
        return $this->id;
    }

    public function setId($id) {
        $this->id = $id;
    }

    public function getAlias() {
        return $this->alias;
    }

    public function setAlias($alias) {
        $this->oldAlias = $this->alias;
        $this->alias = $alias;
    }

    public function getOldAlias() {
        return $this->oldAlias;
    }

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

    public function setEmail($email) {
        $this->oldEmail = $this->email;
        $this->email = $email;
    }

    public function getOldEmail() {
        return $this->oldEmail;
    }
}


You don't need the Schema classes at all. Breaking your application into these layers has several advantages:

  • Fewer classes than your current design (User and UserStorage)



  • Each class does one thing, and does it well



  • User: The Domain Model representing rows in a database. Additional methods can be added to this class to encapsulate business logic, such as changing email address, etc.



  • UserStorage: Implements the CRUD operations of the database, dealing strictly with the User domain model



  • Each identifying field for the User domain model has its own UserStorage method. Strongly defined interfaces in the data access layer will be easier to maintain than the fuzzy interface of "search by id, alias or email but not more than one"



  • Removes an unnecessary external dependency to Whelp\Commons\Schema\Schema from your application

Code Snippets

interface UserStorageInterface
{
    public function findById($id);
    public function findByAlias($alias);
    public function findByEmail($email);
    public function update($user);
    public function create($user);
    public function remove($user);
}
class UserStorage implements UserStorageInterface
{
    public function findById($id) {
        $data = // ... execute query and find by id

        return $this->mapToUser($data);
    }

    public function findByAlias($alias) {
        $data = // ... execute query and find by alias

        return $this->mapToUser($data);
    }

    public function findByEmail($email) {
        $data = // ... execute query and find by email

        return $this->mapToUser($data);
    }

    private function mapToUser($data) {
        $user = new User($data['id']);
        $user->setAlias($data['alias']);
        $user->setEmail($data['email']);

        return $user;
    }

    public function create($user) {
        // INSERT
        $user->setId(/* new Id */);
    }

    public function update($user) {
        // UPDATE
    }

    public function remove($user) {
        // DELETE
    }
}
class User
{
    private $id;
    private $alias;
    private $oldAlias;
    private $oldEmail;
    private $email;

    public __construct($id = null) {
        if (isset($id)) {
            $this->setId($id);
        }
    }

    public function getId() {
        return $this->id;
    }

    public function setId($id) {
        $this->id = $id;
    }

    public function getAlias() {
        return $this->alias;
    }

    public function setAlias($alias) {
        $this->oldAlias = $this->alias;
        $this->alias = $alias;
    }

    public function getOldAlias() {
        return $this->oldAlias;
    }

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

    public function setEmail($email) {
        $this->oldEmail = $this->email;
        $this->email = $email;
    }

    public function getOldEmail() {
        return $this->oldEmail;
    }
}

Context

StackExchange Code Review Q#78188, answer score: 2

Revisions (0)

No revisions yet.