Полезные практики написания поддерживаемого кода на PHP

Привет, меня зовут Алексей и я должен признаться, я PHP разработчик. Последние несколько лет плотно занимаюсь проектами на symfony и решил поделиться с сообществом практиками, которые стараюсь соблюдать при работе.

Многие из них довольно спорные, для дискуссии добро пожаловать в комментарии.

1. Зависимости в di указываем явно

Явное лучше неявного. При описании классов не полагаемся на магию symfony di. Вообще мне кажется, что autowiring довольно вредная практика. При явном описании гораздо проще и быстрее при чтении кода понимать какие зависимости использует класс.

Плохо:

App\Services\MyService: ~

Хорошо:

app.services.my_service:
    class: App\Services\MyService
    arguments:
        - '@doctrine.orm.entity_manager'
        - '@app.services.my_service.dependency'

2. В DI подставляем реализации, а не интерфейсы

Так повышается читабельность и поощряется множественные имплементации интерфейсов.

Плохо:

app.services.my_service:
    class: App\Services\MyService
    arguments:
        - '@App\OtherService\Producer\ProducerInterface'

App\OtherService\Producer\ProducerInterface:
    alias: App\OtherService\Producer\Amqp

Хорошо:

app.services.my_service:
    class: App\Services\MyService
    arguments:
        - '@app.other_service.producer'

app.other_service.producer:
    class: App\OtherService\Producer\Amqp

3. Не надо пробрасывать в классы весь контейнер целиком

Плохо:

final class MyService implements MyServiceInterface
{
    public function __construct(private readonly ContainerInterface $container) {}
    public function doSomething()
    {
        $producer = $this->container->get('app.other_service.producer');
        $producer->publish();
    }
}

Хорошо:

final class MyService implements MyServiceInterface
{
    public function __construct(private readonly ProducerInterface $producer) {}
    public function doSomething()
    {
        $producer->publish();
    }
}

4. Каждый класс закрывается интерфейсом

Позволяет разрабатывать отталкиваясь от интерфейса. Если в месте использования
класса по какой-то причине не подходит его реализация, его легко заменить другой.

final class MyService implements MyServiceInterface
{
    // ...
}

5. Чем меньше методов в интерфейсе, тем лучше.

Большие интерфейсы сложнее поддерживать и реализовать. Нарушается принцип единственной ответственности.

Плохо:

interface SomeEntityInterface
{
    public function getId(): int;
    public function getName(): int;
    public function getAge(): int;
}

Хорошо:

interface IdAwareInterface
{
    public function getId(): int;
}
interface NameAwareInterface
{
    public function getName(): int;
}
interface AgeAwareInterface
{
    public function getAge(): int;
}

6. В интерфейсах указываем какие exception могут быть выброшены

Кроме этих exceptions никакие другие выбрасываться не должны. Если у интерфейса нет блока @throws - значит в реализации все exception должны быть обработаны.

7. Максимально строгая типизация

Используем strict_types=1. Типы указываем максимально жестко.

Плохо:

public function doSomething(mixed $id)
{
    return $id;
}

Хорошо:

public function doSomething(int $id)
{
    return $id;
}

8. Флаги в аргументах функций

Нарушается принцип единственной ответственности. Отдельные классы проще тестировать, можно использовать в разных сценариях с помощью композиции.

Плохо:

final class SomeService {
    public function doSomething($force = false)
    {
        if ($force) {
            return $this->doForce()
        }
        
        return $this->do()
    }
}

Хорошо:

final class ForceService {
    public function doSomething()
    {
        return $this->doForce();
    }
}

9. Количество аргументов в функции максимум 3

Если аргументов много - значит метод выполняет слишком много действий, нарушается принцип единственной ответственности. Это сложно тестировать и поддерживать.

Плохо:

final class SomeService {
    public function doSomething($own, $two, $three, $four)
    {
    }
}

10. Не мутировать объекты переданные в метод/функцию

Это неочевидное поведение для клиентского кода и легко приводит к ошибкам в процессе развития и изменения кода.

Плохо:

$item = new Item();
$this->calculateCount($item);

public function calculateCount(Item $item): void
{
    $count = 0;
    // some logic
    $item->setCount($count);
}

Хорошо:

$item = new Item();
$item->setCount($this->calculateCount());

public function calculateCount(): int
{
    $count = 0;
    // some logic
    return $count;
}

11. Заполнение DTO через конструктор с именованными аргументами, readonly свойствами, без get/set методов

Такая запись читабельна за счёт именованных аргументов, нет лишнего кода get/set, такая dto иммутабельна за счёт readonly.

Хорошо:

final class Item
{
    public function __construct(
        public readonly int $id,
        public readonly int $count,
        public readonly string $originId,
    ) {
    }
}

$item = new Item(
    id: 1,
    count: 2,
    originId: 'aaa-bbb-ccc',
);

12. Разбиваем большие классы и методы

Класс меньше ~150 строк, метод меньше ~30 строк. Это значительно упрощает чтение кода.

13. Unit тесты всегда

На любой код обязательно пишем unit тест (кроме dto). Если есть skiptests - то обязательно добавляем ссылку на то место, где это протестировано.

/**
 * @skiptests tested in {@see \App\SomeOthreService}
 */
final class SomeServiceTest extends TestCase {
}

14. Тестируем поведение, а не детали реализации

Тест должен проверять все варианты поведения модуля, но не должен тестировать внутреннюю реализацию этого поведения.
Не надо в тесте проверять вызов логгера, аргументы исходящих вызовов, вызовы приватных методов, если только в этом не заключается основная логика работы класса. Stubs и dummy заглушки намного предпочтительнее mocks и fakes.

Плохо:

final class SomeServiceTest extends TestCase {
    public function testDoSomething()
    {
        $logger = $this->createMock(LoggerInterface::class);
        $logger->expects($this->once())
            ->method('info')
            ->with('some message');
        $service = new SomeService($logger);
        
        $service->doSomething();
    }
}

Хорошо:

final class SomeServiceTest extends TestCase {
    public function testDoSomething()
    {
        $service = new SomeService(
            $this->createMock(LoggerInterface::class),
        );
        
        $service->doSomething();
    }
}

15. Композиция, а не наследование

Наследование это максимально сильная связь между классами.
Композиция в подавляющем большинстве случаев предпочтительнее.

Плохо:

final class SomeService extends BaseLogic {
}

Хорошо:

final class SomeService {
    public function __construct(private readonly BaseLogic $logic) {}
    public function execute()
    {
        $this->logic->doSomething();
    }
}

16. По умолчанию указываем final для классов

Такое правило позволяет избежать проблем с наследованием и поощряет использование композиции.

final class SomeService {
}

17. Не работаем с ассоциативными массивами, только с объектами

Приходится писать много проверок, сложно расширять. Используем JMS\Serializer\SerializerInterface и
JMS\Serializer\ArrayTransformerInterface.

Плохо:

$id = $data['id'];

Хорошо:

$id = $data->getId();

18. Соблюдаем закон Деметры

Класс должен взаимодействовать только с известными ему модулями, не взаимодействовать с незнакомцами.

Плохо:

$client = $this->clientFactory->create();
$client->send();

Хорошо:

$this->clientDecorator->send();

19. Минимальная цикломатическая сложность

Код должен быть максимально плоским. Чем меньше сложность, тем проще читать код и понимать, что он делает.
Иногда можно жертвовать микрооптимизациями ради читаемости.

Плохо:

foreach ($items as $item) {
    if ($item->isAvailable()) {
        $availableItems[] = $item;
    }
    if ($item->isAcvite()) {
        $activeItems[] = $item;
    }
}

Хорошо:

$availableItems = $this->filterAvailableItems($items);
$activeItems = $this->filterActiveItems($items);