shrikeh/php-coding-bible

PHP Coding bible and associated sniffs

0.3.2 2026-06-17 16:23 UTC

Requires

• php: >=8.2
• slevomat/coding-standard: ^8.4
• squizlabs/php_codesniffer: ^4.0

Requires (Dev)

• ext-xdebug: *
• roave/security-advisories: dev-latest
• shrikeh/testing-metapackage: >=0.3
• symfony/dotenv: ^8.0

MIT

• Barney Hanlon

README

Bible of how we want to write maintainable code and applications.

Ethos

While the Zen of Python is, well, about Python, our code can also benefit from the wisdom of Tim Peters:

• Beautiful is better than ugly.
• Explicit is better than implicit.
• Simple is better than complex.
• Complex is better than complicated.
• Flat is better than nested.
• Sparse is better than dense.
• Readability counts.
• Special cases aren't special enough to break the rules.
• Although practicality beats purity.
• Errors should never pass silently.
• Unless explicitly silenced.
• In the face of ambiguity, refuse the temptation to guess.
• There should be one-- and preferably only one --obvious way to do it.
• Although that way may not be obvious at first unless you're Dutch.
• Now is better than never.
• Although never is often better than right now.
• If the implementation is hard to explain, it's a bad idea.
• If the implementation is easy to explain, it may be a good idea.
• Namespaces are one honking great idea -- let's do more of those!

Overarching Concepts

Don't. Ship. Shit.

"You don't guess that your code works; You know that it works"

- Uncle Bob

Here's the talk. Required viewing.

Code without tests is not production-ready: it's as simple as that. Your tests should mean you know it works before it's deployed. That may mean refactoring code, but we are professionals and the job isn't supposed to be easy.

The Low-Level stuff

Write your code to be:

• Clean!
• PSR-12 compliant
• SOLID
• Testable
• Maintainable

Tests are first-class citizens in your application

Think about it. They're the reason you don't have to write so much documentation, how you know you aren't breaking somebody else's code (and that they can't break yours). They need to be fast and light, atomic enough they can run in parallel, and be easily maintained. So yes, they are equally as valuable as the application code in knowing you're done. Treat them with the same care you would for application code (which is why they should get linted and quality checked as well).

Classes that are easy to test are easy to use and maintain

A core advantage of TDD isn't about tests but how writing code to pass tests makes you keen to easily setup the Subject Under Test, and that the SUT has a limited number of methods and permutations that require testing.

Favour Composition over Inheritance

There should be a very good reason to extend a class. Inheritance causes all sorts of problems with tests, overriding methods, etc. Make your classes final and use collaborators.

Classes Should Self-Validate On Construction

It should be impossible for an object to be created in an invalid state. Objects should self-validate upon construction to ensure they are meaningful and to capture early problems.

# Bad
class Refund
{
    public function __construct(private float $amount)
    {}
}

# Good
final readonly class Refund
{
    public function __construct(private float $amount)
    {
        if (0 >= $amount) {
            // Refunds must be above zero.
            throw RefundNotPositiveException::create($amount);
        }
    }
}

A constructor shouldn't have to normalize data, just validate it

In the case where an object could be created from multiple types of data, it's better to make the __construct() method private and instead use Static Named Constructors. The static methods can then normalize the data, while the true constructor validates it:

final readonly class PdfPath
{
    public static function fromString(string $path): self
    {
        return new self (new SplFileObject($path));
    }
    
    public static function fromFileInfo(SplFileInfo $fileInfo): self
    {
        return new self($fileInfo->openFile());
    }
    
    private function __construct(private SplFileObject $pdfPath)
    {
        if (!$this->pdfPath->isReadable()) {
        // ...
        }
    }
}

The "Rule" Of Three

It's "Rule" rather than a Rule, because it's more like The Pirate's Code.

A method, including a constructor, should have a maximum of three arguments to it.

Collaboration is Key

Collaborators, be they aggregates or providers of functionality, provide the building blocks of good code. They allow the above "rule" to be more easily followed, aid in testing, maintainability, and reuse.

A simple Aggregate example for use within a Customer:

namespace Shrikeh\Customer;

final readonly class ContactDetails
{
    public function __construct(
        private Address $address, 
        private EmailAddress $emailAddress,
        private TelephoneNumber $telephoneNumber,
    ) {
    
    }
    
    // ... accessors
}

Because the above class is using Value Objects, which self-validate themselves, the aggregate needs no validation itself. This then could be used as an argument to a Customer constructor.

But how should you access these aggregated details? Well...

Practice the Law of Demeter

"When one wants a dog to walk, one does not command the dog's legs to walk directly; instead one commands the dog which then commands its own legs."

- Wikipedia

The Law of Demeter is a principle of least knowledge; that is, the properties and structure of an object should not be replied upon by something using it. Instead, the object should have a method that does the work for the external user:

# Bad
$address = $customer->getProfile()->getHomeAddress();

# Good
$address = $customer->homeAddress();

However, be mindful of delegate wrecks, where the external object has to have many methods to delegate to the underlying object. In such cases, it may be better to refactor, as the smell is in what the object is being asked.

Failure should be exceptional

Classes should do what is expected of them or throw a specific Exception. Don't return true; it is expected to work!

Remember: if you are relying on a collaborator, and it throws an Exception to use the previous Exception:

final readonly class DbalCustomerRegistryRepository
{
//...
    public function registerCustomer(Customer $customer): void
    {
        try {
            $this->client->save($customer->getName());
        } catch (ClientException $exc) {
            throw UnableToRegisterCustomerException::create($customer, $exc);
        }
    }
}

Concerned the above doesn't return an ID? Why do you need one - your application has already specified the UID ahead of time via the use of UUIDv5 or UUIDv6.

Expected failure should be unexceptional

That said, not every failure is exceptional. PHP's union types exist for a reason: a customer not being found is not surprising, a payment being declined is part of the normal flow, and a search returning no results is entirely expected. For these cases, an explicit return type — CustomerDetails|null, or a dedicated result value object — forces the caller to handle the failure path at compile time rather than discovering a missing catch in production. Reserve exceptions for things that genuinely shouldn't happen; use the type system for things that might.

Comments are a code smell

Think about it: why is your code so complex you need to explain it? Strong variable names and typing, combined with equally strong method names and brevity, should not require War & Peace to describe them. Functional testing with Gherkin, combined with unit testing obeying the principle of Specification By Example, should provide enough information.

There is, however, a meaningful distinction between comments that describe what code does and comments that capture why it exists. The former is the smell: if the code cannot speak for itself, fix the code. The latter is professional due diligence. A hidden regulatory constraint, a workaround for a third-party library bug, a non-obvious invariant that will silently break under an apparently sensible refactor — no amount of expressive naming conveys these, and no test will warn the next developer they are about to violate them.

Deleting that context is how you rediscover the same problem two years later and wonder why on earth anyone wrote it that way.
The rule: never comment what. Be diligent about commenting why — if the reason would not be immediately apparent to a competent reader approaching the code cold, write it down.

Set is Evil

Objects should be immutable, and created in a specific state which they carry for their lifetime. If you must use set, it should return a new instance with the new value and leave the original unchanged:

final readonly class Foo
{
    public function setBar(string $bar): self
    {
        return new self($bar);
    }
}

However, a more useful name (rather than setX) is with (Wither) pattern):

final readonly class Foo
{
    public function withBar(string $bar): self
    {
        return new self($bar);
    }
}

Get is Evil

Or at least, getters are not necessary for simply returning property values from PHP 8.1, following the addition of public readonly properties:

final class Foo
{
    public function __construct(
        public readonly string $bar
    ) {
    }
}

$foo = new Foo('sample string');
echo $foo->bar; // sample string

replaces

final class Foo
{
    private string $bar;
    
    public function __construct(
        string $bar
    ) {
        $this->bar = $bar;
    }
    
    public function getBar(): string
    {
        return $this->bar;
    }
}

$foo = new Foo('sample string');
echo $foo->getBar(); // sample string

Why?

The principal reason is honesty: a getter that simply returns $this->bar adds no behaviour; it is ceremony. public readonly is explicit about what it is — a value — whereas a getter implies there might be logic within it, creating a false impression in the reader's mind. There is one trade-off worth knowing: public readonly bakes the property name into every caller. If the internal representation later needs to change — the property is renamed, or its value becomes computed rather than stored — that is now a breaking change. For stable value objects this is rarely a concern; for classes whose internal representation is likely to evolve, a method provides a seam to absorb that change. Use your judgement.

Better still, make the entire class readonly wherever possible:

final readonly class Foo
{
    public function __construct(
        public string $bar
    ) {
    }
}

Just a note that, should you use getters because direct property access is undesirable (say, to match an interface) get can infer set. There isn't much need to name a method getAddress() over just calling it address() - from the caller's perspective, it is irrelevant if the value returned was a property, calculated or fetched from some store, so the get part of the method name adds nothing.

New is Evil

There are only a few things that should create other things:

• An object should be able to create a new instance of itself (ie to follow the practice of immutability above)
• A dedicated factory, ideally meeting an interface.
• Dependency Injection containers

Let's imagine a database repository matching the following contract:

interface CustomerDetailsRepository
{
    public function fetchCustomerDetails(CustomerId $customerId): CustomerDetails|null;
}

Now, an implementation that essentially handles the query and Exception handling, without having to know how to create the CustomerDetails:

final readonly class DBCustomerDetailsRepository implements CustomerDetailsRepository
{
    public function __construct(private Client $client, private CustomerDetailsFactory $customerDetailsFactory)
    {
        
    }

    public function fetchCustomerDetails(CustomerId $customerId): CustomerDetails|null
    {
        try {
            $row = $this->queryCustomerDetails($customerId);
            
            return $this->customerDetailsFactory->build($row);
        } catch (SomeClientException $exc) {
            // ... throw a Repository exception
        }
    }

    private function queryCustomerDetails(): SomeDbRow
    {
        //...
    }
}

The above is going to be far easier to mock, test, and maintain.

Make variables and methods private by default

You should only make the minimum number of methods public to fulfill the contract of external use.

Prefer class constants over magic numbers

You know how it is. Months after you wrote a wonderful algorithm that is blisteringly fast and your peers adore you for, you now have to go back into it with a new team member.

final class AwesomeAlgorithm
{
    public function calc(float $marketVolatility, float $weight): float
    {
        return (2.17 ** $marketVolatility) - (1 - $weight);
    }
}

Why is it 2.17, they ask innocently? You panic, not remembering what this magic number is. You see the future all too clearly now: how they will see through your facade of professionalism, how your team will mock you, your family turn their backs on you, and LinkedIn will close your profile. As the sweat pours down your forehead, desperately trying to remember what any of the calculation means, you can tell it is already too late as the first drizzle of perspiration crashes against the keys of your keyboard, and you smell the burning of electronics.

If only you had written it like this:

final class AwesomeAlgorithm
{
    private const float LONG_TERM_VOLATILITY_YIELD = 2.17;

    public function calc(float $marketVolatility, float $weight): float
    {
        return (self::LONG_TERM_VOLATILITY_YIELD ** $marketVolatility) - (1 - $weight);
    }
}

Prefer Enums over multiple class constants of the same "thing"

Having replaced both your predecessor and their moisture-damaged keyboard, you resolve to not make the same mistakes.

Here's a (real) extract from a class written before PHP had enums:

class HsSomeBankCashTx extends DumbActiveRecord
{
    //...

    public const TLA_CODE_TRANSFER = 'TFR';
    public const TLA_CODE_DIRECT_DEBIT = 'DDR';
    public const TLA_BILL_PAYMENT = 'BBP';
    public const TLA_CODE_BANK_GIRO_CREDIT = 'BGC';
    public const TLA_CODE_FASTER_PAYMENT_CREDIT = 'FPC';
    public const TLA_CODE_FASTER_PAYMENT_DEBIT = 'FP';
    public const TLA_CREDIT = 'CR';
    public const TLA_CODE_FASTER_PAYMENT_TRANSFER = 'FT';
    public const TLA_STANDING_ORDER = 'STO';
    //...

You keep your cool, and you refactor the class by extracting these constants into an Enumerable:

enum SomeBankTlaCode: string
{
    case Transfer = 'TFR';
    case DirectDebit = 'DDR';
    case BillPayment = 'BBP';
    case BankGiroCredit = 'BGC';
    case FasterPaymentCredit = 'FPC';
    case FasterPaymentDebit = 'FP';
    case Credit = 'CR';
    case FasterPaymentTransfer = 'FT';
    case StandingOrder = 'STO';
}

Enforce strict typing with declare(strict_types=1)

Newer versions of PHP allows scalar types declarations.

Unfortunately these types can still be coerced into wrong types, take as an example:

<?php

function myFunc(string $str): void 
{	
	echo $str;	
}

myFunc(123);

See https://3v4l.org/6YfXoK

This will simply output 123

Now we add declare(strict_types=1);

<?php

declare(strict_types=1);

function myFunc(string $str): void 
{	
	echo $str;	
}

myFunc(123); 

See https://3v4l.org/smJf1

And we now get:

Fatal error: Uncaught TypeError: myFunc(): Argument #1 ($str) must be of type string, int given

in php >= 8 and for php 7 we get:

Fatal error: Uncaught TypeError: Argument 1 passed to myFunc() must be of the type string, int given

So practice safe coding and always use declare(strict_types=1);