A demonstrational example of a Data mapper ORM

Question

I want to create an educational example of a Data mapper type ORM. You know, it is always interesting to know how the stuff works, but when you take a fully functional software package, it's just impossible to understand its core principles.

If you take even a relatively concise implementation, an Atlas ORM for example, and try to dig it out in order to understand what a Data mapper is, you'd simply give up, unable to tell what is related to the basic pattern and what is added to satisfy other patterns/good practices/real life issues.

So my goal is to create a simplest Data Mapper possible, just to demonstrate the principle, implementing basic CRUD operations. I wanted to make it less than 100 lines of code and succeeded, with help of two default considerations:

• An unique identifier for the record is assumed to be called "id"
• A table name is equal to the lowercase form of a base class name.

I'd like to know is it possible to make it better, so, the questions are:

• how to make it better compliant to the original Data Mapper pattern
• how to make it closer to the real life usage (in its current form it's hardly usable at all)
• any other suggestions or improvements you might have

The code and the usage example are below:

<?php
include 'init.php';

$pdo->query("create temporary table dmtestuser (id int primary key auto_increment, name varchar(25))");

$dm = new PrimitiveDM($pdo);

class dmTestUser {}

$user = new dmTestUser();
$user->name = "Joe";
$dm->save($user);

$dm->delete($user);

$user = new dmTestUser();
$user->name = "Bob";
$dm->save($user);

$id = $user->id;

$user = $dm->find('dmTestUser', $id);
$user->name = "Jane";
$dm->save($user);

$userList = $dm->findBySql('dmTestUser', "SELECT * FROM dmtestuser");
var_dump($userList);


class PrimitiveDM {

    protected $db;
    protected $driver;

    public function __construct(\PDO $pdo, $driver = 'mysql')
    {
        $this->db = $pdo;
        $this->driver = $driver;
    }

    public function query($sql, $params = [])
    {
        $stmt = $this->db->prepare($sql);
        $stmt->execute($params);
        return $stmt;
    }

    protected function escapeIdent($ident)
    {
        switch ($this->driver)
        {
            case 'mysql':
                return "`".str_replace("`", "``", $ident)."`";

            default:
                throw new \Exception("You must define escape rules for the driver ($driver)");
        }
    }

    public function findBySql($class, $sql, $params = [])
    {
        return $this->query($sql, $params)->fetchAll(\PDO::FETCH_CLASS, $class);
    }

    public function find($class, $id)
    {
        $table = $this->escapeIdent(strtolower(basename($class)));
        $sql = "SELECT * FROM $table WHERE id = ?";
        return $this->query($sql, [$id])->fetchObject($class);

    }

    public function delete($object)
    {
        $table = $this->escapeIdent(strtolower(basename(get_class($object))));
        $sql = "DELETE FROM $table WHERE id = ?";
        $this->query($sql, [$object->id]);

    }
    public function save($object)
    {
        $table = $this->escapeIdent(strtolower(basename(get_class($object))));
        $properties = get_object_vars($object);
        $params = array_values($properties);

        if (!empty($object->id))
        {
            $set = '';
            foreach($properties as $name => $value)
            {
                $set .= $this->escapeIdent($name) . " = ?,";
            }
            $set = substr($set, 0, -1);
            $sql = "UPDATE $table SET $set WHERE id = ?";
            $params[] = $object->id;
            $this->query($sql, $params);

        } else {

            $names = '';
            foreach($properties as $name => $value)
            {
                $names .= $this->escapeIdent($name) . ",";
            }
            $names = substr($names, 0, -1);
            $values = str_repeat('?,', count($params) - 1) . '?';
            $sql = "INSERT INTO $table ($names) VALUES ($values)";
            $this->query($sql, $params);
            $object->id = $this->db->lastInsertId();
        }
    }
}

Answer 1

I don't fully understand the desire for a "generic" data mapper. Oftentimes, flexibility leads to complexity and fragility in your application. Take for example your delete() method. It just arbitrarily accepts whatever object is passed and starts deleting things from the database. What is to stop someone from just creating a new object named similarly to a table you don't want deletes happening against (like a configuration table) and arbitrarily destroying records in the database?

This flexibility can lead to bad habits like doing SELECT * queries, overuse of publicly accessible properties on your objects, and using PDO::fetch_object() which can circumvent class constructor behavior.

I guess as an educational exercise like this perhaps has value, but for a real-world production-level application, I think this class is pretty much a non-starter.

I know it might seem like a lot of extra code to have to build "model" classes for each of your database entities, but I would strongly suggest doing just this, as you will likely find over time that a one-size-fits all mapper such as this just becomes a mess when trying to adapt it to the various use cases you may have in a more complex application.

I DO think that what you have may be informative towards such classes, as a lot of what you have written might be well-aligned with an abstract base model class which is extended for each entity type. A "search by id" function for example, is probably pretty common across all classes, where you might find that an update method might be hard to implement in a base class and should perhaps be defined as an abstract method there.

Some more specific notes follow.

---

I don't understand the need for $driver and escaping you are doing here. If you go with an inheritance model where inheriting class specify the tables/columns they related to, you can get away from all this unnecessary escaping of database object names.

---

Consider using more meaningful/specific variable and method names. For example

$db => $pdo (since it holds a PDO object)
query() => prepare() (since this method does not perform a query at all, but rather creates a prepared statment)
find() => findById()

---

Should your save() method actually be broken up into create() and update() methods? I would think that a programmer should be explicit about what they are expecting to happen here. And you should not ever let the class get set up to where id would be empty if the record has been persisted to the database.

---

Your code really only considers happy path. What if prepared statements fail? What if select queries return zero rows?

---

You are leaking implementation details outside this class. Why would you be returning PDOStatement objects to the caller like in your query() method. If this class wants to be a mapper, it should ONLY be a mapper, not a general purposes class for preparing statements.

---

findBySql() is an odd method. It accepts arbitrary query input, so how to you know that the query is even a SELECT vs. some other type of query. Why would a mapper class even need to accept arbitrary SQL?

---

Why not use PDO::quote() for escaping string rather than your own logic?

---

Your public methods really do nothing to validate that the arguments being passed are suitable to be working with. You should never get so far as to prepare a statement on the database (a relatively expensive operation) if you have not validated that you have reasonable values to work with.

I would suggest you add guarding code at the first few lines in this methods to validate the input and fail with InvalidArgumentException or similar before you get to executing method logic. Fail early and fail loudly.

---

You repeat this code: strtolower(basename(get_class($object))) in several methods. Perhaps your class needs a getTableNameFromClass() method that can be used in your functions, or this should be part of escapeIdent().

---

I challenge your thinking that a class name and table name should be the same with the exception of camel casing. Most relational databases treat database object names (tables, columns, etc.) in a case-insensitive fashion or could have problems if the database is run on different OS that vary on case sensitivity with regard to the physical file names in which the data is stored. So many consider it best practice to simply remove any potential chance for error and use snake_case in relational databases.

Answer 2

Here's a funny question for you: if this exact same code returned arrays, instead of objects, would it still be a data mapper? ;-)

Regardless, it should be kind-of easy to specify alternative primary key columns. I suppose something like:

public function setPrimary($class, $col) {
    $this->primary[$class] = $col;
}

And then any time you need the primary column:

$primary = $this->primary[$class] ?? 'id';

I presume something similar would be possible for mapping class names to table names:

public function setTable($class, $table) {
    $this->table[$class] = $table;
}

// ...

$table = $this->table[$class] ?? $class;

As far as real-life usage: ah, there's the rub. "Real life" usage is going to require a lot more than 100 lines (or 1000 lines) can give. :-/

Answer 3

I'm not sure if it was intentional but it appears that for the most part PSR-2 is being followed, except that in save() the opening braces for the control structures are on the next line for the if statements but on the same line for the else statement.

if (!empty($object->id))
{
    ...
} else {

And similarly for the foreach.

Perhaps it feels like the code is self-documenting enough but it wouldn’t hurt to add docblocks above each class and method...

---

The code within save() to generate the UPDATE statement generates the variable set with the following lines:

$set = '';
foreach($properties as $name => $value)
{
    $set .= $this->escapeIdent($name) . " = ?,";
}
$set = substr($set, 0, -1); 

Whenever I see code like the last line to remove the last comma, it makes me cringe. Instead of appending to a string the loop could push values into an array and then join them with join():

$set = [];
foreach($properties as $name => $value)
{
    $set[] = $this->escapeIdent($name) . " = ?";
}
$fieldsToSet = join(',', $set);

While it may require slightly more computational power, a functional approach could be used and would require three less lines of code:

$set = implode(',', array_map(function($name) {
    return $this->escapeIdent($name) . " = ?";
}, array_keys($properties)));

And I noticed that if id is set, that will be included in the list of fields to set - while it likely won't lead to changing the value, it may be wise to remove that from the list of fields to set.

And when getting the list of fields to set - a functional approach makes it a single liner, since nothing needs to be appended to each value returned from the method call:

$names = implode(',', array_map([$this, 'escapeIdent'], array_keys($properties)));

That takes a block of 6 lines down to 1! Combined with the change to generate $set this could be a reduction of 9 lines... could allow for expansion of your code and keeping it under 100 lines.

One could argue that $names could be better named - e.g. $fieldsToSet, $setFields, etc.