MJ Convento

/

Enums replaced forty class constants

2,242 words, about 10 min read

The file was called PoStatus.php, it was 312 lines long, and it held 41 public constants: nine current purchase order statuses, six legacy aliases kept alive for rows written before 2019, nine label strings, nine badge classes, and eight transition lists. It also held a static isValid() that nobody called on the write path, and a static label() built out of a switch whose default threw an exception that had been thrown in production exactly twice in three years. Last Monday it became 46 lines and one enum.

PHP 8.1.0 shipped on 25 November. Two weeks later the honest status here is that the internal back office, which is Symfony 5.3 and serves about thirty people in the office, went to 8.1 on 6 December. The larger client application is still on 8.0 and will be until January, because one of its dependencies has an open incompatibility. So this is not a fleet-wide upgrade report. It is a report on one codebase small enough to actually refactor, which turns out to be the right place to learn what enums cost.

Forty-one constants and a switch with a default that threw

Here is the shape of the old file, cut down to what matters:

final class PoStatus
{
    public const DRAFT     = 'draft';
    public const SUBMITTED = 'submitted';
    public const APPROVED  = 'approved';
    public const PARTIAL   = 'partial';
    public const RECEIVED  = 'received';
    public const CANCELLED = 'cancelled';
    // plus 35 more: legacy aliases, labels, badge classes, transition lists

    public static function isValid(?string $status): bool
    {
        return in_array($status, [
            self::DRAFT, self::SUBMITTED, self::APPROVED,
            self::PARTIAL, self::RECEIVED, self::CANCELLED,
        ], true);
    }

    public static function label(?string $status): string
    {
        switch ($status) {
            case self::DRAFT:
                return 'Draft';
            case self::SUBMITTED:
                return 'Submitted for approval';
            // six more arms
            default:
                throw new InvalidArgumentException(
                    sprintf('unknown status "%s"', $status)
                );
        }
    }
}

Everything wrong with this is in the type declarations. label() takes ?string, because a string is what the database hands you, and so every caller in the application also passes a string around. Nothing in the language stopped anyone writing $po->setStatus('aproved'), and somebody had, in a fixture, in 2020. isValid() existed precisely because the type system was not doing its job, and being a separate call, it was only invoked where somebody had remembered.

What the backed enum gave back

The replacement is one file. Enums are built on classes, so they can implement interfaces and carry methods, and a backed enum adds a scalar equivalent for round-tripping to the database:

namespace AppEnum;

enum PoStatus: string implements Labelled
{
    case Draft = 'draft';
    case Submitted = 'submitted';
    case Approved = 'approved';
    case Partial = 'partial';
    case Received = 'received';
    case Cancelled = 'cancelled';

    public function label(): string
    {
        return match ($this) {
            self::Draft => 'Draft',
            self::Submitted => 'Submitted for approval',
            self::Approved => 'Approved',
            self::Partial => 'Partially received',
            self::Received => 'Received',
            self::Cancelled => 'Cancelled',
        };
    }

    /** @return list<self> */
    public function allowedNext(): array
    {
        return match ($this) {
            self::Draft => [self::Submitted, self::Cancelled],
            self::Submitted => [self::Approved, self::Cancelled],
            self::Approved => [self::Partial, self::Received, self::Cancelled],
            self::Partial => [self::Received, self::Cancelled],
            self::Received, self::Cancelled => [],
        };
    }

    public function canTransitionTo(self $next): bool
    {
        return in_array($next, $this->allowedNext(), true);
    }
}

Four things about that code are worth pulling out.

The match has no default. It does not need one, because the RFC guarantees the case list is closed and every case is a singleton that passes an identity check. If somebody adds a seventh case and forgets an arm, match throws UnhandledMatchError at the exact line, which is a much better failure than a label silently rendering as an empty string. Our static analysis catches it before that, which is the real point.

in_array($next, $this->allowedNext(), true) works for the same reason: each case is one object instance, so PoStatus::Approved === PoStatus::Approved is always true. What does not work is using a case as an array key. Enum cases are objects, so the old [self::DRAFT => [...]] transition map could not be ported directly, which is why it became a match. The RFC notes they are legal keys in a SplObjectStorage or WeakMap, and since cases are never garbage collected those two are effectively the same thing here.

isValid() is gone, and its replacement is the boundary. from() takes the scalar and returns the case or throws a ValueError; tryFrom() returns null instead. So the controller that reads a status out of a query string uses PoStatus::tryFrom($request->query->get('status')) and falls back, and the importer that reads a trusted column uses from() and lets it blow up. When it does blow up the message names both sides, which beats unknown status "": "cancelled_by_ops" is not a valid backing value for enum "AppEnumPoStatus". That is the legacy alias we had forgotten about, found on day one by a test.

And cases() replaced three hand-maintained arrays. The status filter dropdown, the admin form’s choice list and the CSV export header all iterate PoStatus::cases() now. Adding a case updates all three. Net result across the refactor: 41 constants down to 6, 214 lines deleted, 46 added, and one InvalidArgumentException subclass removed entirely.

Doctrine has no idea what an enum is yet

This is the part to check before you promise anyone an afternoon. The current ORM is 2.10.3, released 3 December, and it has no native enum support. The request is issue 9021, opened on 17 September, still open. So there is no enumType on a column mapping and there will not be one this year.

What there is, and has been for years, is the DBAL’s custom mapping type system. You extend Type, implement getName(), getSQLDeclaration(), convertToDatabaseValue() and convertToPHPValue(), and register it. For an enum that is about thirty lines:

namespace AppDoctrineType;

use AppEnumPoStatus;
use DoctrineDBALPlatformsAbstractPlatform;
use DoctrineDBALTypesConversionException;
use DoctrineDBALTypesType;

final class PoStatusType extends Type
{
    public const NAME = 'po_status';

    public function getName(): string
    {
        return self::NAME;
    }

    public function getSQLDeclaration(array $column, AbstractPlatform $platform): string
    {
        return $platform->getStringTypeDeclarationSQL(['length' => 32]);
    }

    public function convertToDatabaseValue($value, AbstractPlatform $platform): ?string
    {
        if ($value === null) {
            return null;
        }

        if (!$value instanceof PoStatus) {
            throw ConversionException::conversionFailedInvalidType(
                $value,
                self::NAME,
                [PoStatus::class, 'null']
            );
        }

        return $value->value;
    }

    public function convertToPHPValue($value, AbstractPlatform $platform): ?PoStatus
    {
        if ($value === null || $value instanceof PoStatus) {
            return $value;
        }

        return PoStatus::tryFrom((string) $value)
            ?? throw ConversionException::conversionFailed($value, self::NAME);
    }
}

Registered under doctrine.dbal.types and set as the column type on the entity, that works, and the entity property gets a real PoStatus type declaration. Two rough edges. Types in the DBAL are flyweights with no state, so one class per enum: six enums means six of these files, which is exactly the boilerplate a native enumType option would delete. And parameter binding is not automatic everywhere. In query builder andWhere() calls where Doctrine cannot infer the field, we pass $status->value rather than the case, because debugging a silent empty result set is worse than four extra characters. When native support lands I expect to delete all six type classes in one commit.

Serialisation: JSON, the normalizer, and Twig

JSON was the easy half. A backed enum serialises to its scalar value with no work at all, so json_encode(['status' => PoStatus::Approved]) gives {"status":"approved"}, and the existing API contract did not move. A pure enum is the opposite: it has no scalar, so json_encode() fails with JSON_ERROR_NON_BACKED_ENUM and the message “Non-backed enums have no default serialization”, or throws a JsonException if you pass JSON_THROW_ON_ERROR. Both of our new enums are backed, partly for the database and partly for this.

The Symfony Serializer is a version problem, not a design problem. Support for backed enumerations is listed in the 5.4 changelog, and 5.4 shipped on 29 November, four days after PHP 8.1. This app is on 5.3 until January, so there is a fourteen line normalizer in src/Serializer with a comment saying which commit deletes it.

Twig is the awkward one. Twig 3.3 has no enum-aware function, so you cannot name a case from a template the way you name a class constant in PHP. What does work is reading the case: name and value are read-only properties on the case object, and methods are just methods, so this is fine:

<td data-status="{{ order.status.value }}">{{ order.status.label }}</td>

{% if order.status is same as(constant('App\Enum\PoStatus::Cancelled')) %}
    <p>This order was cancelled on {{ order.cancelledAt|date('d M Y') }}.</p>
{% endif %}

That constant() call works for a non-obvious reason: cases are implemented internally as class constants, and Twig’s constant function is a thin wrapper over PHP’s constant(), which resolves them. It is also unreadable, and a fully qualified class name in a template is a refactor hazard. We used it in two places and then replaced both with a boolean passed in from the controller. Templates should not be doing type comparisons.

The rest of 8.1, and what we skipped

Readonly properties went on fourteen DTOs in an afternoon and are the second best thing in this release. Read the restrictions before you start, because two of them will catch you. A readonly property must be typed and cannot have a default value, and the clone-based wither pattern stops working: $clone = clone $this; $clone->x = $x; throws, because the cloned property is already initialised. The RFC is explicit that this is by design. Our one wither became a new static(...) call and is better for it. Note also that interior mutability survives: a readonly property holding an object still lets you modify that object, which is worth saying out loud to anyone who thinks the modifier makes a graph immutable.

never went on four methods, all of them helpers that end in a redirect or a throw, and it immediately earned its keep by making a piece of dead code visible in a controller. readonly and never are new keywords in 8.1, never being fully reserved, and neither collided with anything of ours. First-class callable syntax, $this->normaliseRow(...) instead of [$this, 'normaliseRow'], replaced thirty-one callable arrays and is the change I have to stop myself making everywhere. New in initializers took one constructor from a null default plus a null coalesce down to Logger $logger = new NullLogger().

Skipped: fibers, because nothing in this stack is asynchronous and I am waiting to see what the library authors build on them rather than using them directly. Pure intersection types, because we have not found a case yet and they cannot be mixed with union types. Final class constants, filed under nice.

The deprecations that will cost a sprint

One deprecation dominates the list: passing null to a non-nullable parameter of an internal function. Scalar parameters of built-in functions have always been quietly nullable, and 8.1 begins closing the gap with userland functions. The manual’s example is str_contains("foobar", null), which now says Passing null to parameter #2 ($needle) of type string is deprecated. In a codebase that reads nullable database columns and hands them to trim(), strlen(), htmlspecialchars() and explode(), that is not one fix. Our importer produced 1,190 of these notices in a single run:

[2021-12-07 09:14:22] php.DEPRECATED: Passing null to parameter #1 ($string)
of type string is deprecated at /var/www/app/src/Import/RowNormaliser.php line 44

Be clear on what is not deprecated, because I have already had this argument twice. Implicit nullable parameter types in your own code, function f(int $x = null), are untouched in 8.1. That is a separate question for a later release. What did change in 8.1 is stricter: an optional parameter declared before a required one is now always treated as required, and calling with named arguments throws ArgumentCountError where 8.0 only emitted a notice.

Three more from the same page cost us real time. htmlspecialchars() and friends now default to ENT_QUOTES | ENT_SUBSTITUTE instead of ENT_COMPAT, so single quotes get escaped where previously nothing happened, which broke two snapshot tests and one email template. PDO’s MySQL driver now returns native integers and floats from emulated prepared statements instead of strings, which found four === '1' comparisons. And strftime() is deprecated in favour of date() or IntlDateFormatter::format(); we had twenty-three calls, all in old report builders, all locale-dependent, and that one is still open.

Next: three more enums queued for January, all of them the same shape, all of them currently string columns guarded by a static validator. The one I am not doing yet is the shipment status on the client application, because it is 8.0 until that dependency moves, and because a fourteen case enum whose values came from a partner’s API is the sort of thing where I would rather have native Doctrine support before I start.

Sources