A demonstrational example of a Data mapper ORM

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:

```
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 = [])
{

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.

StackExchange Code Review Q#155312, answer score: 8