A lightweight CRUD, based on the Table Data Gateway pattern

Question

In order to reduce the boilerplate code, I decided to write a lightweight database helper, based on the Table Data Gateway pattern (or at least the way I understand it) with the aim of using it for a simple CRUD application.

Writing INSERT and UPDATE queries is always a nuisance, repeating all those ranks of column names, parameter names, binding parameters and such. So I decided to write a simple class that does all the job under the hood, and accepts/returns simple arrays, without going down the rabbit hole of creating a full featured ORM.

My aim was also to make this class as secure as possible, hence the table and field names are always hardcoded.

It consists of one prototype class,

abstract class BasicTableGateway
{
    protected $db;
    protected $table;
    protected $fields;
    protected $primary = 'id';

    public function __construct(\PDO $db)
    {
        $this->db = $db;
    }
    public function listBySQL(string $sql, array $params): array
    {
        $stmt = $this->db->prepare($sql);
        $stmt->execute($params);
        return $stmt->fetchAll();
    }
    public function list(int $page = 1, int $limit = 50): array
    {
        $page--;
        $offset = $page * $limit;
        $stmt = $this->db->prepare("SELECT * FROM `$this->table` LIMIT ?,?");
        $stmt->execute([$offset,$limit]);
        return $stmt->fetchAll();
    }
    public function read($id): ?array
    {
        $ret = $this->listBySQL("SELECT * FROM `$this->table` WHERE `$this->primary`=?", [$id]);
        return $ret ? reset($ret) : null;
    }
    public function update(array $data, int $id): void
    {
        $this->validate($data);

        $params = [];
        $set = "";
        foreach($data as $key => $value)
        {
            $set .= "`$key` = ?,";
            $params[] = $value;
        }
        $set = rtrim($set, ",");
        $params[] = $id;
        $sql = "UPDATE `$this->table` SET $set WHERE `$this->primary`=?";
        $this->db->prepare($sql)->execute($params);
    }
    public function insert($data): int
    {
        $this->validate($data);

        $fields = '`'.implode("`,`", array_keys($data)).'`';
        $placeholders = str_repeat('?,', count($data) - 1) . '?';

        $sql = "INSERT INTO `$this->table` ($fields) VALUES ($placeholders)";
        $this->db->prepare($sql)->execute(array_values($data));

        return $this->db->lastInsertId();

    }
    public function delete($id)
    {
        $sql = "DELETE FROM `$this->table` WHERE `$this->primary`=?";
        $this->db->prepare($sql)->execute([$id]);
    }
    protected function validate($data)
    {
        $diff = array_diff(array_keys($data), $this->fields);
        if ($diff) {
            throw new \InvalidArgumentException("Unknown field(s): ". implode($diff));
        }
    }
}

and children classes where related properties are set to operate certain tables:

class UserGateway extends BasicTableGateway {
    protected $table = 'users';
    protected $fields = ['email', 'password', 'name', 'birthday'];
}

which is then used like this

$data = [
    'email' => '[email protected]',
    'password' => 'hash',
    'name' => 'Fooster',
];
$id = $userGateway->insert($data);
$user = $userGateway->read($id);
echo json_encode($user),PHP_EOL;

$userGateway->update(['name' => 'Wooster'], $id);
$user = $userGateway->read($id);
echo json_encode($user),PHP_EOL;

$users = $userGateway->list();
echo json_encode($users),PHP_EOL;

$userGateway->delete($id);

The code works all right for a simple CRUD but I'd like to know whether it can be improved or may be there are some critical faults I don't realize.

Answer 1

• In list(), because $page is a single-use variable, I don't know that it is worth decrementing. Perhaps write just one line as:

  $offset = ($page - 1) * $limit;
  

  but then again $offset is a single-use variable. If you prefer the declarative coding style, leave it as is. Alternatively, you could just write the arithmetic directly in the execute() call. (either way, don't forget to add the space between offset and limit arguments)

  $stmt->execute([($page - 1) * $limit, $limit]);
  

  or because of precedence, you can pre-decrement to avoid parentheses:

  $stmt->execute([--$page * $limit, $limit]);
  

  or as stated in comments, "list can internally call listBySql" from @hjpotter92:

  public function list(int $page = 1, int $limit = 50): array
  {
      return listBySQL("SELECT * FROM `$this->table` LIMIT ?,?", [--$page * $limit, $limit]);
  }
  

• Please add the int type declaration to the $id argument in read(). I think that read() can be reduced to this:

  return reset($this->listBySQL("SELECT * FROM `$this->table` WHERE `$this->primary`=?", [$id])) ?: null;
  

  because if fetchAll() delivers an empty array, then reset() will return false in which case the Elvis operator can return the desired null value. This is more of a style preference alternative rather than a functional gain -- the only real benefit is removing the $ret variable.

• I would prefer that update() return int instead of void (even if your demo doesn't seek this information). In my projects, I always return the number of affected rows. This is integral in knowing if the query was actually successful in making changes or if there were no changes from the valid query. I find this to be as important as insert() returning the new id.

• Please add the array type declaration on $data in insert() and remove the blank line after returning the new id in insert().

• Please add the int type declaration on $id in delete().

• There is some inconsistency regarding when you declare a $sql variable and when you write the sql string directly into the prepare() call. I think you should pick one style or the other. My leanings, of course, bias toward removing single-use variables.

• validate()'s incoming argument should have an array type declaration. The implode() call in the thrown exception could use some glue -- a comma at minimum.