Skip to content

paysera/php-style-guide

Folders and files

NameName
Last commit message
Last commit date

Latest commit

 

History

43 Commits
 
 
 
 
 
 

Repository files navigation

Paysera PHP style guide

Paysera PHP style guide extends PSR-1 and PSR-12, so code must also follow these standards to be compatible with this style guide.

Guide is divided into several parts:

  • PHP Basics. This defines additional style rules for the code, some restrictions for basic PHP usage;
  • Main patterns. This describes patterns and architecture decisions that must be followed;
  • Symfony related conventions. As Paysera use Symfony as the main framework, this chapter describes rules to be used when developing for this framework;
  • Composer conventions. Describes usage rules related with composer: releasing libraries or requiring ones.

Table of contents

PHP Basics

Basics

PHP code-level

For our libraries we use lowest PHP version that is used in any of currently actively maintained projects.

Globals

We do not use global variables, constants and functions.

Files

We put every class to it’s own file.

Exceptions for code style usage

If we modify legacy code where some other conventions are used, we can use the same style as it is used there.

Code style

Commas in arrays

If we split array items into separate lines, each item comes in it’s own line. Comma must be after each (including last) element.

<?php
['a', 'b', 'c'];
[
    'a',
    'b',
    'c', // notice the comma here
];

Why?

  • each element in it's own line allows to easily scan all the elements - more understanding, less bugs;
  • comma after last element allows to easily reorder or add new elements to the end of the array.

Splitting in several lines

If we split statement into separate lines, we use these rules:

  • &&, || etc. comes in the beginning of the line, not the end;

  • if we split some condition into separate lines, every part comes in it’s separate line (including first - on the new line, and last - new line after). All of the parts are indented.

Some examples:

<?php

return ($a && $b || $c && $d);

return (
    $a && $b
    || $c && $d
);

return ((
    $a
    && $b
) || (
    $c
    && $d
));

return (
    (
        $a
        && $b
    )
    || (
        $c
        && $d
    )
);

return ($a && in_array($b, [1, 2, 3]));

return (
    $a
    && in_array($b, [1, 2, 3])
);

return ($a && in_array(
    $b,
    [1, 2, 3]
));

return ($a && in_array($b, [
    1,
    2,
    3,
]));

return ($a && in_array(
    $b,
    [
        1,
        2,
        3,
    ]
));

return (
    $a
    && in_array(
        $b,
        [
            1,
            2,
            3,
        ]
    )
);

This is wrong:

<?php
return [
        'a',    // use 4 spaces, not 8 here
        'b',
    ];

Chained method calls

When making chain method calls, we put semicolon on it’s own separate line, each of chained method calls is indented and comes in it’s own line.

Example:

<?php
return $this->createQueryBuilder('a')
    ->join('a.items', 'i')
    ->andWhere('i.param = :param')
    ->setParameter('param', $param)
    ->getQuery()
    ->getResult()
;  // semicolon here

Why? Same as with arrays: allows to easily scan all the calls, reorder or add new ones when needed.

Constructors

We always add () when constructing class, even if constructor takes no arguments:

<?php
$value = new ValueClass();

Variable, class and function naming

Full names

We use full names, not abbreviations: $entityManager instead of $em, $exception instead of $e.

Class naming

We use nouns for class names.

For services we use some suffix to represent the job of that service, usually *er:

  • manager

  • normalizer

  • provider

  • updater

  • controller

  • registry

  • resolver

We do not use service as a suffix, as this does not represent anything (for example PageService).

We use object names only for entities (value objects), not for services (for example Page).

Interface naming

We always add suffix Interface to interfaces, even if interface name would be adjective.

Why? If we have base class witch implements the interface, we would have name clash. For example, ContainerAware and ContainerAwareInterface.

Property naming

We use nouns or adjectives for property names, not verbs or questions.

<?php

declare(strict_types=1);

class Entity
{
    private bool $valid;       // NOT $isValid
    private bool $checkNeeded; // NOT $check
}

Method naming

We use verbs for methods that perform action and/or return something, questions only for methods which return boolean.

Questions start with has, is, can - these cannot make any side-effect and always return boolean.

For entities we use is* or are* for boolean getters, get* for other getters, set* for setters, add* for adders, remove* for removers.

We always make correct English phrase from method names, this is more important that naming method to 'is' + propertyName.

<?php
interface EntityInterface
{
    public function isValid(): bool;
    public function isCheckNeeded(): bool;
    public function getSomeValue(): string;
    public function canBeChecked(): bool;
    public function areTransactionsIncluded(): bool;

    // WRONG:
    public function isCheck(): bool;
    public function isNeedsChecking(): bool;
    public function isTransactionsIncluded(): bool;
    public function getIsValid(): bool;
}
<?php
interface ControllerInterface
{
    public function canAccess(int $groupId): bool;

    /**
     * @throws AccessDeniedException
     */
    public function checkPermissions(int $groupId): void;
}

Order of methods

We provide methods and fields in the following order:

  • constants
  • static fields
  • fields
  • constructor
  • constructing class methods
  • static methods
  • class methods

Constructing class methods are those used in service construction phase, usually by dependency injection container.

Why no ordering by visibility? protected and private methods usually are used from single place in the code, so ordering by functionality (versus by visibility) makes understanding the class easier. If we just want to see class public interface, we can use IDE features (method names ordered by visibility or/and name, including or excluding inherited methods etc.)

Directories and namespaces

Singular namespaces

We use singular for namespaces: Service, Bundle, Entity, Controller etc.

Exception: if English word does not have singular form.

No *Interface namespaces

We do not make directories just for interfaces, we put them together with services by related functionality (no ServiceInterface namespace).

Different namespaces and service names

We do not use service names for namespaces, instead we use abstractions. For example, namespace should be UserMerge or UserMerging, not UserMergeManager.

Comparison order

If we compare to static value (true, false, null, hard-coded integer or string), we put static value in the right of comparison operator.

Wrong: null === $something

Right: $something === null

Why? Speak clearly to be understood correctly, you should. Yeesssssss.

Namespaces and use statements

If class has a namespace, we use use statements instead of providing full namespace. This applies to PhpDoc comments, too.

Wrong:

<?php

namespace Some\Namespace;

// ...

public function setSomething(\Vendor\Namespace\Entity\Value $value): self;

Right:

<?php

namespace Some\Namespace;

use Vendor\Namespace\Entity\Value;

public function setSomething(Value $value): self;

Usage of PHP features

Condition results

If condition result is boolean, we do not use condition at all.

Do not do this:

<?php
$a = $d && $e ? false : true;

if ($a && $b) {
    return true;
}

return false;

Do this:

<?php
$a = !($d && $e);

return $a && $b;

Logical operators

We use && and || instead of and and or.

Why? and and or have different priorities and commonly leads to unintended consequences

Strict comparison operators

We use === (!==) instead of == (!=) everywhere. If we want to compare without checking the type, we should cast both of the arguments (for example to a string) and compare with ===.

Same applies for in_array - we always pass third argument as true for strict checking.

We convert or validate types when data is entering the system - on normalizers, forms or controllers.

Why? Due to security reasons and to avoid bugs

php > var_dump(md5('240610708') == md5('QNKCDZO'));
bool(true)
php > file_put_contents('/tmp/a', '<?xml version="1.0" encoding="UTF-8"?><B4B xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:noNamespaceSchemaLocation="http://swedbankgateway.net/valid/hgw-response.xsd"><Alert><AccountInfo><IBAN>abc</IBAN></AccountInfo></Alert></B4B>');
php > $a = simplexml_load_file('/tmp/a');
php> var_dump($a == false);
bool(true)
php > var_dump(in_array(1, ['1a2b']));
bool(true)
php > var_dump(in_array(true, ['a']));
bool(true)

Converting to boolean

We use type casting operator to convert between types:

<?php
return (bool)$something;

This should not be used at all. If variable is not boolean already, check it’s available values explicitly.

Comparing to boolean

We do not use true/false keywords when checking variable which is already boolean.

Wrong:

<?php
return $valid === true;
return $valid === false;
return false !== $valid;

Correct:

<?php
return $valid;
return !$valid;

Exception: When variable can be not only boolean, but also int or null.

<?php
return strpos('needle', 'haystack') === false;

Comparing to null

When comparing to null, we always compare explicitly.

<?php

function func(?ValueClass $value = null): ?string
{
    return $value !== null ? $value->getSomething() : null;
}

//or in php8
function func(?ValueClass $value = null): ?string
{
    return $value?->getSomething();
}

We also do not use is_null function for comparing.

Assignments in conditions

We do not use assignments inside conditional statements.

Exception: in a while loop condition.

Wrong:

<?php
if (($b = $a->get()) !== null && ($c = $b->get()) !== null) {
    $c->do();
}
if ($project = $this->findProject()) {

}

Correct:

<?php
$b = $a->get();
if ($b !== null) {
    $c = $b->get();
    if ($c !== null) {
        $c->do();
    }
}

$project = $this->findProject();
if ($project !== null) {

}

Why? We save a few lines of code but code is less understandable - we make several actions at once. Furthermore, as we explicitly compare to null, conditional-assignment statements become complicated.

Unnecessary variables

We avoid unnecessary variables.

Wrong:

<?php

function getSomething(): ?string
{
    $a = get();
    return $a;
}

Correct:

<?php

function getSomething(): ?string
{
    return get();
}

This does not mean that we cannot use them to make code more understandable. For example, this way of variable usage is fine:

<?php

function canModify(ValueClass $object): bool
{
    $rightsGranted = isAdmin() || isOwner($object);
    $objectEditable = isNew($object) && !isLocked($object);

    return $rightsGranted && $objectEditable;
}

Reusing variables

We do not set value to variable passed as an argument.

We do not change the type of any variable.

Instead, in both cases, we create new variable with different name.

<?php

public function thisIsWrong(int $number, string $text, Request $request): void
{
    $number = (string)$number;  // Illegal: we 1) change argument value 2) change it's type
    $text .= ' ';  // Illegal: we change argument value
    $document = $request->get('documentId');
    $document = $this->repository->find($document); // Illegal: we change variable's type
    // ...
}

public function thisIsCorrect(string $numberText, string $text, Request $request): void
{
    $number = (int)$numberText;
    $modifiedText = $text . ' ';
    $documentId = $request->get('documentId');
    $document = $this->repository->find($documentId);
    // ...
}

Why? To understand code better (quicker, easier) and thus to avoid bugs. This also integrated with IDE better so it can give better static analysis support

Unnecessary structures

We avoid unnecessary structures.

Wrong:

<?php
if ($first) {
    if ($second) {
        do();
    }
}

Correct:

<?php
if ($first && $second) {
    do();
}

Static methods

We do use static methods only in these cases:

  • dataProvider in phpunit 10

Converting to string

We do not use __toString method for main functionality, only for debugging purposes.

Why? We cannot throw exceptions in this method, we cannot put it in interface, we cannot change return type or give any arguments - refactoring is impossible (only by changing it to simple method). Also if debugging uses __toString method, we cannot change it to get any additional information.

Double quotes

We do not use double quotes in simple strings.

We use double quotes only under these conditions:

  • Single quote is used repeatedly inside the string
  • Some special symbols are used, like "\n"

If we use double quotes, we do not use auto variable includes ("Hello $name!" or "Hello {$object->name}!").

Visibility

Public properties

We don’t use public properties. We avoid magic methods and dynamic properties - all used properties must be defined.

Example:

<?php

declare(strict_types=1);

class Alpha {
    private int $alpha;      // this must be defined as a property
    public int $beta;       // this is illegal

    public function __construct(int $alpha)
    {
        $this->alpha = $alpha;
    }
}

Method visibility

We prefer private over protected over public as it constraints the scope - it's easier to refactor, find usages, plan possible changes in code. Also IDE can warn about unused methods or properties.

We use protected when we intend some property or method to be overwritten if necessary in extending classes.

We use public visibility only when method is called from outside of class.

Functions

count

We use count instead of sizeof.

is_null

We use compare to null using === instead of is_null function. For example:

if ($result === null) {
    // ...

str_replace

We do not use str_replace if we need to remove prefix or suffix - this can lead to replacing more content unintentionally.

<?php
function removePrefix($text, $prefix)
{
    if (substr($text, 0, strlen($prefix)) === $prefix) { // strpos === 0 is also possible
        $text = substr($text, strlen($prefix));
    }
    return $text;
}

assertSame('Some asd text', removePrefix('asdSome asd text', 'asd'));

We can also use preg_replace for this.

Return and argument types

Return types

We always return value of one type. Optionally, we can return null when using any other return type, too.

For example, we cannot return boolean|string or SuccessResult|FailureResult (if SuccessResult and FailureResult has no common class or interface; if they do, we document to return that interface instead).

We can return ?SomeClass or ?string.

Note: when we are always returning the same class but it's iterable of other classes, we can (and should) provide Collection|SomeClass[]|null as return value.

Argument types

Same applies for any argument.

Passing ID

Exception: When making query from repository, both int|Entity can be taken (object or it’s ID). We give priority to object in this case if it’s available.

Usually this should not be done as it would probably violate some other convention.

Type-hinting optional arguments

If argument is optional, we provide default value for it.

If optional argument is object, we type-hint it with required class and add default to null.

If argument is not optional, but just nullable, we can typehint it with default value null, but when using, we pass null explicitly.

Example:

<?php

declare(strict_types=1);

class Service
{
    public function __construct(SomeService $service, ?LoggerInterface $logger = null)
    {
        //...
    }
}
<?php

declare(strict_types=1);

class Entity
{
    public function setValue(?ValueClass $value): self
    {
        //...
        return $this;
    }
}

$entity->setValue($someValue);
$entity->setValue(null);
// but we do not use $entity->setValue();

Void result

We always return something or return nothing. If method does not return anything ("returns" void), we do not return null, false or any other value in that case.

If method must return some value, we always specify what to return, even when returning null.

Wrong:

<?php
function makeSomething(): bool
{
    if (success()) {
        return true;
    }
}
function getValue(MyObject $object): string
{
    if (!$object->has()) {
        return;
    }
    return $object->get();
}
/**
 * @param int $requestId
 * @return bool
 * @throws PaybackUnavailableException
 */
function payback(int $requestId): bool
{
    makePayback($requestId);
    return true;
}

Correct:

<?php
function makeSomething(): ?bool
{
    if (success()) {
        return true;
    }
    return null;
}
function getValue(MyObject $object): ?string
{
    if (!$object->has()) {
        return null;
    }
    return $object->get();
}
/**
 * @param int $requestId
 * @throws PaybackUnavailableException
 */
function payback(int $requestId): void
{
    makePayback($requestId);
}

Why? If method result is not used and you return something anyway, other programmer may assert that return value is indeed taken into account. For example, return false on failure, even if this failure is not handled anyhow by the functionality that's calling your method.

Correct typehinting for possibly uninitialized properties

If we declare some class property type as not nullable, after that class object construction it should never be or become null.

To rephrase from other perspective – if property is not initialized in the constructor, it's type must be nullable.

This also applies for typehints in PhpDoc of properties.

Examples:

<?php

declare(strict_types=1);

class SomeClass
{
    /**
     * @var int|null    –> this must include `|null`, as `$id` can be uninitialized
     */
    private $id;

    public function getId(): ?int   // return value here must be nullable
    {
        return $this->id;
    }

    /**
     * @return int|null
     */
    public function getIdBefore71()
    {
        return $this->id;
    }

    public function setId(int $id): self    // we can use non-nullable type for setter, though
    {
        $this->id = $id;

        return $this;
    }
}
<?php

declare(strict_types=1);

class SomeClass
{
    private ?Child $child;

    public function getChild(): Child
    {
        if ($this->child === null) {
            throw new RuntimeException('child is not initialized yet');
        }

        return $this->child;
    }

    public function hasChild(): bool
    {
        return $this->child !== null;
    }
}

Type-hinting classes and interfaces

We always type-hint narrowest possible interface which we use inside the function or class.

Why? This allows us to refactor easier. We can just provide another class, which implements the same interface. We can also find real usages quicker if we want to change the interface itself (for example RouterInterface vs UrlGeneratorInterface when we change declaration of RouterInterface::matches).

Dependencies with several interfaces

If we have dependency on service with several responsibilities (which implements several interfaces), we should inject it twice. For example:

declare(strict_types=1);

class ResultNormalizer implements NormalizerInterface, DenormalizerInterface
{
    // ...
}

class MyService
{
    public function __construct(
        NormalizerInterface $normalizer,
        DenormalizerInterface $denormalizer
    ) {
        // ...
    }
    // ...
}

$resultNormalizer = new ResultNormalizer();
$myService = new MyService($resultNormalizer, $resultNormalizer);

Dates

We use \DateTimeImmutable objects to represent date or date and time inside system.

We use \DateTimeInterface where we get the date to be more compatible with functionality that still operates \DateTime objects.

Why? Because immutable version is preventing accidental data modification by design.

Exceptions

Throwing

We never throw base \Exception class except if we don’t intend for it to be caught.

Catching

We never catch base \Exception class except where it’s thrown from vendor code.

In any case, we never catch exception if there are few throwing places possible and we only expect one of them.

Wrong:

<?php

try {
    // code
    // some more code
    $this->service->someDeepMethod();
    // more code here
} catch (\Exception $exception) {
    return null; // not found
}

Why? Because usually in these situations tens or hundreds different situations could have gone wrong but we make assumption that it was that single one. This is how we not only make the system behave incorrect on unexpected failures, but also ignore the real problems by not logging them.

Checking things explicitly

We use only functions or conditions that are designed for specific task we are trying to accomplish. We don’t use unrelated features, even if they give required result with less code.

We avoid side-effects even if in current situation they are practically impossible.

Some examples:

  • we use isset versus empty if we only want to check if array element is defined;

  • we use $x !== '' instead of strlen($x) > 0 - length of $x has nothing to do with what we are trying to check here, even if it gives us needed result;

  • we use count($array) > 0 to check if array is not empty and not !empty($array), as we do not want to check whether $array is 0, false, '' or even not defined at all (in which case IDE would possibly hide some warnings that could help noticing possible bugs).

Why? We avoid side-effects if some related code changes or is refactored. Code is much easier to understand if we see specific checks or function calls instead of something unrelated that happens to give the needed result.

Calling parent constructor

If we need to call parent constructor, we do it as first statement in constructor. For example:

public function __construct(int $arg1, int $arg2)
{
    parent::__construct($arg1);
    $this->setArg2($arg2);
}

Why? Parent class can have some mandatory stuff to do before we can use it's functionality. See following example. This is the only way to call parent constructor in some (probably most) of other languages (etc. JAVA)

Example of parent class, with which calling constructor later would fail:

/**
 * @var int[] $params
 */
private array $params;
public function __construct(int $arg1)
{
    $this->params = [$arg1];
}
public function setArg2(int $arg2)
{
    $this->params[] = $arg2;
}

Traits

The only valid case for traits is in unit test classes. We avoid using traits in our base code.

Why? We use (classical) OOP to refactor functionality to avoid duplicated code.

Arrays

We always use short array syntax ([1, 2] instead of array(1, 2)) where PHP code-level allows it.

Default property values

If we need to define some default value for class property, we do this in constructor, not in property declaration.

Why? Some default values cannot be set when declaring properties, so this way everything is in one place. It especially makes sense when entity has lots of properties.

<?php

declare(strict_types=1);

class MyClass
{
    private const TYPE_TWO = 'two';

    private ArrayCollection $one;
    private string $two;
    private int $three;
    private bool $four;

    private DateTimeImmutable $createdAt;

    public function __construct()
    {
        $this->one = new ArrayCollection();
        $this->two = self::TYPE_TWO;
        $this->three = 3;
        $this->four = false;
        $this->createdAt = new DateTimeImmutable();
    }
}

Scalar typehints

Strict types declaration is required

For newly written files we always add declare(strict_types=1); at the top of the file.

We add it for already existing files if we can guarantee that no negative side-effects would be caused by it. It's safer to type-cast variables in the file to the correct scalar types to keep the consistent behavior.

Always use scalar typehints where appropriate

We always use scalar typehints both for arguments and return types, unless our code with them would not work as intended so we are forced not to use them.

For already existing methods we can add scalar typehints but we're responsible to guarantee that any place in the project that has strict types declaration and calls this method always passes the correct type as an argument. We could add typecast in the places where we're not guaranteed to keep the consistent behavior.

If we add scalar typehint for any argument in a public method in our library, this means that major release is needed. Scalar typehints can be always safely added to return values if needed, though.

<?php

declare(strict_types=1);

class Something
{
    public function increment(int $alpha): int
    {
        return $alpha + 1;
    }
}

// some old interface from some old library we have to implement
interface SomeOldInterface
{
    /**
     * @param string $withSomething
     * @return Result[]
     */
    public function doOldStuff($withSomething);
}

class OurShinyNewClass implements SomeOldInterface
{
    /**
     * @param string $withSomething
     * @return Result[]
     */
    public function doOldStuff($withSomething): array // we can not declare argument type, but we can narrow return type
    {
        return [];
    }
}

Void typehints

For functions and methods that do not return value, we should typehint return value as : void.

<?php

declare(strict_types=1);

class SomeClass
{
    public function doSomething(): void // we must typehint return value as `void` here as method does not return anything
    {
        echo "Hi!";
    }
}

Comments

PhpDoc on methods

No PhpDoc on fully strictly typed methods

If PhpDoc does not add any other information that's already provided with typehints, we don't use PhpDoc at all.

Wrong:

/**
 * @param Client $client
 * @param string $type
 *
 * @return Money
 */
public function getLimit(Client $client, string $type): Money;

public function setNumber($number);

Correct:

public function getLimit(Client $client, string $type): Money;

public function setNumber(int $number): self;

Why? Any comment needs to be maintained – if we add parameter, change parameter or return type, we must also update the PhpDoc. If PhpDoc does not add any additional information, it just duplicates information that's already provided.

Additional information in PhpDoc

If method description, parameter descriptions or any other PhpDoc tags other than @param and @return are used we will add a full PhpDoc like the following:

/**
 * Gets a day limit starting from midnight.
 *
 * @see http://example.com/
 *
 * @param Client $client
 *
 * @return Money
 */
public function getDayLimit(Client $client): Money;

PhpDoc on arrays

When we take as an argument or return an array of strictly typed elements, we should add a PhpDoc to describe the type of elements in the array.

/**
 * @param Client[] $clients
 *
 * @return Money
 */
public function getDayLimitForClients(array $clients): Money;

PhpDoc contents

If we use PhpDoc comment, it must contain all information about parameters, return type and exceptions that the method throws. If method does not return anything, we skip @return tag.

sometimes scalar types are not autocompleted by IDE, but must be provided by this requirement. Example: @param string $param1

PhpDoc on properties

We use PhpDoc on properties that are not injected via constructor.

We do not put PhpDoc on services, that are type-casted and injected via constructor, as they are automatically recognised by IDE and desynchronization between typecast and PhpDoc can cause warnings to be silenced.

Fluid interface

If method returns $this, we use typehint : self that IDE could guess correct type if we use this method for objects of sub-classes.

Multiple available types

If we return or take as an argument value that has few types or can be one of few types, we provide all of them separated by |.

<?php
/**
 * Description of the method - optional
 *
 * @param string $param1 description of param1. Description is optional, type is required (string)
 *
 * @return SomeObject[]|Collection|null Optional description of return type
 */
public function getSomething(string $param1)
{
    // ...
    return $collection;
}

Why? This way we

  1. know if null value can be returned or passed as an argument;
  2. can use methods with auto-completion from both of specified types. This is very convenient for collections - we can use $collection->count() and foreach ($collection as $a) {$a->someMethod();}

Deprecations

If method is deprecated, we mark this in PhpDoc as @deprecated with description or additional @see with new method to use. For example:

@deprecated use DI to create and inject this service
@deprecated
@see \Acme\Component\SomeComponent::someMethod

Additional comments

We do not add file comments.

Why? We use class comments - they are used if PhpDoc is auto-generated from PHP code, also can be used by IDE.

We do not use @package, @namespace, @date or @author annotations anywhere.

Why? Namespace is provided by PHP keyword. Both date and author can be seen via annotations and becomes stale really quickly. After refactoring, the only name in the file can be totally irrelevant.

We do not use @inheritdoc.

Why? It does not contain information for programmer. For IDE, no PhpDoc at all has the same results.

Comment styles

We use multi-line /** */ comments for method, property and class annotations.

We use single-line /** @var Class $object */ annotation for local variables. We always avoid this if possible - usually PhpDoc is enough (sometimes it's missing, for example in vendor code).

We can use // single line comments in the code.

We do not use /* */ or # comments at all.

Why no multi-line comment? We try to keep functions small and comment them in PhpDoc. If some things are to be explained in function body, single line comments should be enough. If we want to comment-out something, we just delete it (and use VCS to revert if needed) or disable functionality in configuration.

Some examples:

<?php

class NewsletterSender
{
    /**
     * @var ProcessorInterface[] this must be multiline - 3 lines, not 1
     */
    private $processors;

    public function sendNewsletter(Newsletter $newsletter): void
    {
        foreach ($this->processors as $processor) {
            /** @var Mail $mail this must be single line - 1 line, not 3 */
            $mail = $processor->createNamedMail($newsletter->getName());
            //..
        }

        // we can add single line comments to explain behavior
    }
}

We always prefer refactoring code to be understandable over adding some comments to explain it's behavior. Most of the cases, code should be clear enough to not need any comments at all.

Why should we avoid comments? They are technical debt - if you cannot keep it updated, it gets stale real quick. When you cannot trust some of the comments, you don't know which ones you can really trust.

IDE Warnings

We always avoid IDE warnings if possible (it's not possible only when there is an IDE bug or some similar case). We provide @var PhpDoc comment if IDE cannot guess the type of variable and we cannot fix method declaration for type to be correctly guessed (vendor code etc.)


Main patterns

Thin model

We use Plain Value Objects (we name them Entities) for moving information/data around functionality.

We do not put any logic into those objects - all logic is handled outside, in services.

This allows to change and configure behaviour in different context.

Services without run-time state

In general we try to make services without run-time state. That is, once configured, it should not change it’s behaviour in run-time.

This does not apply to specific services, which have, for example, to collect some events and redispatch them in the end of request etc. - in those cases where collecting the state in run-time is their primary objective.

Instead of:

<?php
class BadExample
{
    private string $name;
    public function setManagerName(string $name): self
    {
        $this->name = $name;

        return $this;
    }
    public function search(): void
    {
        // do something with $this->name
    }
}

we use:

<?php
class GoodExample
{
    public function search(string $managerName): void
    {
        // ...
    }
}

Why? When services become context-aware unintentionally, this makes unpredicted side-effects. More bugs, which can be very hard to debug and/or test. Much more test scenarios. Much harder to refactor.

This is similar to using globals - if we store run-time context, we become unaware of all the things that can influence the behavior.

Composition over inheritance

We always try to use composition instead of inheritance.

If we want to make some abstract method - we inject interface into constructor with that method.

This helps to follow single responsibility principle. Also this often allows to configure object by injecting different functionality without explosion of classes. If we have 2 abstract methods and 2 possible logic for each of them, we would need 4 classes with 8 methods total, also having some duplicated code. If we inject services, we only need 4 classes with 1 method each with no duplicated code.

Also, this allows to test code much easier, as we can test each functionality separately.

Services (objects) over classes, configuration over run-time parameters

This is related to composition over inheritance - we try to configure each service instead of hard-coding any of parameters in the code itself.

For example, instead of:

class ServiceAlpha
{
    public function doSomething(): void
    {
        $alpha = Manager::getInstance();
        $alpha->doSomething('a');
    }
}
class ServiceBeta
{
    public function doSomething(): void
    {
        $alpha = Manager::getInstance();
        $alpha->doSomething('b');
    }
}
$alpha = new ServiceAlpha();
$beta = new ServiceBeta();

we use:

class Service
{
    private Manager $manager;
    public function __construct(Manager $manager)
    {
        $this->manager = $manager;
    }
    public function doSomething(): void
    {
        $this->manager->doSomething();
    }
}
$alpha = new Service(new Manager('a')); // we use Dependency Injection Container for creating services, this is just an example
$beta = new Service(new Manager('b'));

This allows to reuse already tested code in different scenarios just by reconfiguring it.

Of course, we still need to test the configuration itself (functional/integration testing), as it gets more complicated.

Constant usage

We do not store configuration in constants.

Why? As configuration can change, it is not constant. Possible situation: const SOFT = 'soft'; const HARD = 'soft';

Small, understandable methods

We try to write code that is self-explanatory. This implies writing small methods - we should understand what the method does by looking at it's code. We also try to name methods by their meaning, which also helps understanding the code.

Dependencies

We always take into account component or bundle dependencies.

No unnecessary dependencies

No unnecessary dependencies between components or bundles must be created.

If there is some abstract functionality, it must not depend upon it’s implementations.

No circular dependencies

No circular dependencies between components or bundles must be created.

Services

Service creation

Services can be created only by factory classes or dependency injection container. We do not create services in other services not dedicated to do that (for example, in constructor).

Changing entity state

Use of managers

We make changes to entity state in managers or some similar designated services.

Manager can make the following actions:

  • check initial entity state before changing it, validate it;
  • change the state;
  • dispatch event;
  • make some additional actions.

If state is changed in a controller or some other place, duplicated code can occur when functionality is required in another place. With duplicated code, different behavior can occur, which eventually leads to bugs.

Methods for changing state

We prefer concrete methods instead of one abstract method to change state.

This allows us to see available actions and make our code clearer: no switch statements, event maps, complex validation rules, nested `if`s or similar magic.

<?php

declare(strict_types=1);

class Manager
{
    // simple short methods with clear logic:
    public function accept(Request $request): void
    {
        $this->assertPending($request);
        $request->setStatus(Request::STATUS_ACCEPTED);
        $this->eventDispatcher->dispatch(RequestEvents::ACCEPTED, new RequestEvent($request));
    }
    public function deny(Request $request): void
    {
        $this->assertPending($request);
        $request->setStatus(Request::STATUS_DENY);
        $this->eventDispatcher->dispatch(RequestEvents::DENIED, new RequestEvent($request));
    }

    // more complicated and totally unclear from interface
    public function changeStatus(Request $request, string $status): void
    {
        $this->assertPending($request);
        switch ($status) {
            case Request::STATUS_ACCEPTED:
                $eventName = RequestEvents::ACCEPTED;
                break;
            case Request::STATUS_DENY:
                $eventName = RequestEvents::DENIED;
                break;
            default:
                throw new \InvalidArgumentException('Invalid status provided ' . $status);
        }
        $request->setStatus($status);
        $this->eventDispatcher->dispatch($eventName, new RequestEvent($request));
    }
}

To make example more complicated, we can imagine that different validation groups must be provided for validator for each of the given status and some other event class should be provided if request is denied. Furthermore, denial could take one more argument - DenialReason $reason.

Data types

We use objects to represent data when possible. For example, if data can be represented by both scalar value and an object (like date), we use object. When several variables belong to single item, we use an object representing them and do not pass or operate on these separately (like money object versus amount and currency as separate variables).

We always try to use single representation of that data item - we avoid having multiple classes which represent the same object in the system (for example Transfer as DTO in common library and as an Entity - these could both co-exist, but we try to have an Entity as soon as possible in the system).

We normalize data as soon as possible in the system (input level) and denormalize it as later as possible (output level). This means that in service level, where our business logic resides, we always operate on these objects and not their concrete representations.

Identifier usage

We operate with objects inside services, not their identifiers. Objects must be resolved by their identifiers in controller, normalizer or similar input layer. In business objects (entities or simple DTOs, like Filter) we already have other objects, taken from database.

If we do not find object by identifier, we throw exception and return 404 as a response (or specific error code with 400).

Date and time

In some cases we use integer UNIX timestamp to represent date with time. When creating DateTimeImmutable, we must not use constructor with @ as it ignores the time zone of the server.

For example:

<?php
$createdAt = new DateTimeImmutable();
$createdAt->setTimestamp($data['created_at']);
$entity->setCreatedAt($createdAt);

Money

Always amount and currency, never only amount.

One-to-many Relation in Services

Structure

If there is functionality that can be implemented in different ways and should be selected at runtime, we create Interface for it and use Manager (or separate Registry class) to collect all those services that implement it.

From code, we do not call those services directly, we call Manager which chooses the correct service and redirects call to it.

This way if Interface changes, we only have to make changes in the Manager. Also, if we need to do some action every time some method is called, we can add it to manager, no need to put it to all those services.

Example:

declare(strict_types=1);

class Manager
{
    /**
     * @var ProviderInterface[]
     */
    private $providers = [];

    public function addProvider(ProviderInterface $provider, string $providerKey): self
    {
        $this->providers[$providerKey] = $provider;

        return $this;
    }

    public function doSomething(Data $data): string
    {
        if (!isset($this->providers[$data->getProviderKey()])) {
            throw new RuntimeException();
        }

        return $this->providers[$data->getProviderKey()]->doSomething($data);
    }
}

Tags

We use dependency injection container tags to add all those services to the manager.

Event dispatcher

We use events to notify about some result or to optionally change behaviour before making some action.

We do not use events to actually make some action happen, which is mandatory for the place which dispatches the event.

We should not make assumptions about listeners of some event. If we require a result from event listeners, this indicates that we should refactor functionality and use interfaces with tags or some similar solution.

Handling sensitive values