[Part 3/100] Eloquent sucks

As the README says:

illuminate / database

[READ ONLY] Subtree split of the Illuminate Database component (see laravel/framework)

Illuminate Database

The Illuminate Database component is a full database toolkit for PHP, providing an expressive query builder, ActiveRecord style ORM, and schema builder. It currently supports MySQL, Postgres, SQL Server, and SQLite. It also serves as the database layer of the Laravel PHP framework.

Usage Instructions

First, create a new "Capsule" manager instance. Capsule aims to make configuring the library for usage outside of the Laravel framework as easy as possible.

use Illuminate\Database\Capsule\Manager as Capsule
$capsule = new Capsule;

$capsule->addConnection([
    'driver' => 'mysql',
    'host' => 'localhost',
    'database' => 'database',
    'username' => 'root',
    'password' => 'password',
    'charset' => 'utf8',
    'collation' => 'utf8_unicode_ci',
    'prefix' => '',
]);

// Set the event dispatcher used by Eloquent models... (optional)
use Illuminate\Events\Dispatcher;
use Illuminate\Container\Container

…

View on GitHub

The Illuminate Database component is a full database toolkit for PHP, providing an expressive query builder, ActiveRecord style ORM, and schema builder. It currently supports MySQL, Postgres, SQL Server, and SQLite. It also serves as the database layer of the Laravel PHP framework.

Part of this toolkit available is called Eloquent, which is the ORM for the Laravel Framework.

tl;dr, it sucks because...

• Active Record itself sucks
• their actual implementation of Active Record is also bad and exposes a lot
• models represent a table row, but can do so much beyond that
• there's like 10 different ways to access model properties
• expectations aren't managed around null vs '0' vs false
• the illuminate/database code quality is low
• mutators are gross (get<X>Attribute is unobviously a proxy method)
• and of course it sucks because artisan code comes at an unknown cost to the unsuspecting

Bad Active Record implementation

For those unfamiliar with the design pattern, Active Record is a design pattern commonly associated with ORMs for relational databases.

It's often seen as subjective to dislike Active Record, but it objectively mixes concerns; which is something all developers should at very least understand what they're trading off.

Even better, Laravel's implementation of Active Record publicly exposes implementation details which is unnecessarily encouraging poor development choices to the unaware.

<?php

namespace App\Models;

use Illuminate\Database\Eloquent\Model;

class User extends Model
{
    /**
     * Get the phone associated with the user.
     */
    public function phone()
    {
        return $this->hasOne(Phone::class);
    }
}

$user = User::find(1);
$phone = $user->phone;

^ Standard innocent usage of Eloquent, fetching a related model.

In order for $user to resolve the relational 1:1 model Phone, it needs to perform a database query. Again, standard for Active Record - but in other words, $user has been tightly coupled with your actual database connection.

With that in mind, and a single Eloquent model ($user) here's a list of functionality you can perform directly from your model instance:

You could... create a fresh QueryBuilder for subsequent unrelated queries

$user = User::find(1);

$freshQueryBuilder = $user
  ->getConnection()
  ->query();
  ->from(
    (new UnrelatedModel())
      ->getTable()
  );

$freshQueryBuilder->from('table_name')->get();

or, you could access the schema manager and drop your database... from the model

$user = User::find(1);

$user
  ->getConnection()
  ->getDoctrineSchemaManager()
  ->dropDatabase(
    $post
    ->getConnection()
    ->getDatabaseName()
  )

or, you could build a query, and execute in Doctrine DBAL

$user = User::find(1);

$statement = $user
  ->newQuery()
  ->where('is_activated', 1)
  ->toSql();

$user
  ->getConnection()
  ->getDoctrineConnection()
  ->executeStatement($statement);

or, you could serialize something into JSON... from your model

$json = $user
  ->getGrammar()
  ->prepareBindingForJsonContains(new class implements JsonSerializable {
  public $coolData = [
  'something' => 'cool',
  ];

  public function jsonSerialize() {
    return $this->coolData;
  }
}
});

// $json => '{"something": "cool"}'

obviously no one will ever do this (please don't). The point is this functionality should not be exposed.
Active Record is bad enough without some unstable APIs sprinkled on top.

I'm not going to start on Entity Mapping vs Active Record today, but if you value testability and separating concerns I would recommend Laravel-doctrine.

• http://www.laraveldoctrine.org/

Cyclomatic complexity

After seeing the shear size of https://github.com/illuminate/database/blob/427babd19deffaf7a288abd7cdbbcd2aaa86144d/Connection.php I decided to run a PHPMD Codesize scan on the whole illuminate/database repository

Highlights:

• The class Model has 2073 lines of code.
• The class Model has 112 public methods and attributes.
• The class Builder has 1653 lines of code.
• The class Connection has 35 public methods.
• The class Connection has 43 non-getter and setter-methods.
• The class Connection has 70 public methods and attributes.
• The class Connection has 1385 lines of code.
• The method addCastAttributesToArray() has an NPath complexity of 865.
• The method withAggregate() has an NPath complexity of 1304.
• The class Factory has 37 non-getter- and setter-methods. ... and much much more.

don't forget the many magic method usages too.

I would strongly recommend looking into PHPMD, and what NPath & cyclomatic complexity means if it's new to you.

Eloquent and associated database code is essentially complex, hard to work with bloated code. On a high level what this means for framework users is that your underlying database will find it hard to introduce more functionality, and find it hard to improve existing functionality, but will also be more likely to introduce new bugs, because complexity is already high.

Magical schema-awareness

If you have a table of people, such as:

 CREATE TABLE Persons (
    PersonID int,
    LastName varchar(255),
    FirstName varchar(255),
    isOverweight TINYINT(1),
    City varchar(255)
); 

and a model of

use Illuminate\Database\Eloquent\Model;

class Person extends Model {
    /**
     * The table associated with the model.
     *
     * @var string
     */
    protected $table = 'Persons';

    public function getIsOverweight($value)
    {
        return !empty($value) ? bool($value) : null;
    }
}

Laravel just miraculously happens know how to resolve the values from table columns.

$person = Person::find(1);
$person->City; // 
$person->isOverweight; // Nullable BOOL column

It's very neat 'artisan' code isn't it? The cost of this magic is:

• IDE auto-completion
• Loss of static analysis
• magic methods usage __get = slower code
• Unknown types (is it a string? bool? null? what does a non-existent column return? it's unobvious without deep diving)

Don't forget you can also access model values via...

$person['isOverweight'];
$person->getAttribute('isOverweight');
$person->getAttributes()['isOverweight'];
$person->getAttributeValue('isOverweight');
$person->attributesToArray()['City'];
$person->getOriginal('City');
$person->getRawOriginal('City');

Do you feel safe enough to trust Laravel will run the mutator (type cast '1' to true) with all of these methods? A sane person would say no, I don't trust like that.

Mutators themselves deeper the wound of model accessors, not knowing what types to expect, and handling null vs 0, etc.

Allowing such dynamic functionality, and duplicate ways to 'get' something is another hidden cost that may be forever unobvious to those choosing to stick within the Laravel ecosystem.

Redaction

After writing this post I came across a tweet by Laravel creator, which actually justifies some of my pain points

basmatitree

@basmatitree

@taylorotwell Eloquent's Model class has 1400 lines, uses 7 traits and every model needs to extend it. That's not what I would call simple!

22:45 PM - 10 Jan 2017

Taylor Otwell 🪐

@taylorotwell

@basmatitree then you haven't seen doctrine 😘😂

23:13 PM - 10 Jan 2017

https://en.wikipedia.org/wiki/False_equivalence

Comments

[shayneoneill]

My gripe with Laravel is that it attempts to pull from two inspirations without really understanding either of them. It clearly wants to be ruby on rails, but php doesnt have the metaprogramming chops of ruby, which is the glue of why Rails feels so pleasant to work with. And its added a lot of the bureacracy of Java, which is exactly the sort of thing rails was created to avoid. The end result is its the worst of both worlds. If you want Java like structure, use Java and reap the benefits of a static typed high performance language. If you want rapid development magic, use Rails. But you cant have both. There are alternatives that actually do seem to have gotten it right. Django is both very enterprise capable, whilst heavily leaning on Pythons rapid dev cycle AND its got 19+ years of experience under its belt (python is a reasonably old language this point, predating PHP, JS, Java and all of them).

I just dont see a viable usecase for Laravel that has any clear advantages over its alternatives.

[Byter-Bit]

It's horrid because there is NOTHING you need it for; that is: there is nothing you can't do with PHP without it, and thus ALL it does is add layers of obscure anti-pattern to your work.
Code and business logic and dev-ops are complex enough without throwing ANOTHER layer of stuff around. Just say NO.

[Andrei Dascalu]

Yep. Also, though, Doctrine doesn’t have that complexity problem anymore (with respect to models at least) since it’s worked a lot on decoupling and models can be straight php classes, no need to extend any base model.