ООП. Как избежать передачи флаг как аргумента функции?

Question

[sgidlev]

Приветствую, разработчики.
Итак, есть некий сервис, который обрабатывает запрос от контроллера на добавление или обновление пользователя.

service

<?php
declare(strict_types=1);

namespace App\Feature\User\Service;

use App\Application\Model\Response\IdResponse;
use App\Feature\User\Entity\User;
use App\Feature\User\Model\UserRequest;
use App\Feature\User\Repository\UserRepository;

readonly class UserService
{
    public function __construct(private UserRepository $repository)
    {
    }

    public function createUser(UserRequest $request): IdResponse
    {
        $user = new User();
        $this->upsertUser($user, $request);

        return new IdResponse($user->getId());
    }

    public function updateUser(int $id, UserRequest $request): void
    {
        $this->upsertUser($this->repository->getById($id), $request);
    }

    public function upsertUser(User $user, UserRequest $request): void
    {
        $user->setName($request->getName());
        $user->setActive($request->getActive());

        // for update request only
        if ($this->isUserNotChanged($user, $request)) {
            // ... some logic
        }

        // for create request only
        if ($this->isUserPhoneExists($user)) {
            // ... some logic
        }

        $this->repository->saveAndCommit($user);
    }

}

Логика происходит в методе upsertUser() и в нем заложена разная логика для обработки запроса на добавление и на обновление данных.

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

1. Самое простое по времени и трудозатрам, но и самое неправильное на мой взгляд, это прокинуть дополнительный аргумент, как флаг.

flag argument

public function upsertUser(User $user, UserRequest $request, bool $isUpdateRequest = false): void
{
    $user->setName($request->getName());
    $user->setActive($request->getActive());

    if ($isUpdateRequest) {
        // for update request only
        if ($this->isUserNotChanged($user, $request)) {
            // ... some logic
        }
    } else {
        // for create request only
        if ($this->isUserPhoneExists($user)) {
            // ... some logic
        }
    }

    $this->repository->saveAndCommit($user);
}

2. Использовать разные методы для создания и обновления юзера.
Способ хороший, правда получается много дублирования кода.

separate methods

public function upsertUserForCreateRequest(User $user, UserRequest $request): void
{
    $user->setName($request->getName());
    $user->setActive($request->getActive());

    // for create request only
    if ($this->isUserPhoneExists($user)) {
        // ... some logic
    }

    $this->repository->saveAndCommit($user);
}

public function upsertUserForUpdateRequest(User $user, UserRequest $request): void
{
    $user->setName($request->getName());
    $user->setActive($request->getActive());

    // for update request only
    if ($this->isUserNotChanged($user, $request)) {
            // ... some logic
    }

    $this->repository->saveAndCommit($user);
}

3.Через интерфейсы у меня не получилось красиво реализовать, т.к. логика в функциях принимает разное количество аргументов и соответственно разные аргументы.

Собственно, как правильно обработать логику?

Comments

[mayton2019]

Если PHP поддерживает lambda-функции как аргумент, то можно эту логику просто передавать в родительскую функцию.

Правда здесь кода и так немного. Я не думаю что кода станет меньше. Просто он будет нормализован. Без повторов.

[Дмитрий]

тут напрашивается метод валидации, чтоб не дублировать логику установления значений из реквеста, метод на проверку существования юзера, и метод апсерт превращается в три строчки кода, валидация (возможно превращение в дто), проверка на существование, и на результате проверки инсерт или апдейт
присмотритесь к коду, посмотрите какие строки у вас дублируются, их выносите в общие
такие как

$this->repository->saveAndCommit($user);
//
$user->setName($request->getName());
$user->setActive($request->getActive());
// из этого можно собирать один раз дто из реквеста

Answer 1

[Дмитрий Гординский]

Это нормально что за создание и обновление могут отвечать разные методы.
Куда странней выглядит создание пустого объекта юзера, с последующим его заполнением, только лишь для большего единообразия между обновлением и созданием.

Answer 2

[sgidlev]

Моё решение сейчас такое (на примере обработки класса сотрудника управляющей компании):

Сервис сотрудника управляющей компании (методы, которые обрабатываются из контроллера)

ManagementEmployeeService

<?php
declare(strict_types=1);

namespace App\Feature\Management\Service;

use App\Application\Model\Response\IdResponse;
use App\Feature\Management\Entity\ManagementEmployee;
use App\Feature\Management\Mapper\ManagementEmployeeMapper;
use App\Feature\Management\Mapper\ManagementMapper;
use App\Feature\Management\Model\Employee\ManagementEmployeeDetails;
use App\Feature\Management\Model\Employee\ManagementEmployeeRequest;
use App\Feature\Management\Repository\ManagementEmployeeRepository;
use App\Feature\User\Mapper\UserMapper;

readonly class ManagementEmployeeService
{
    public function __construct(
        private ManagementEmployeeRepository    $employeeRepository,
        private ManagementEmployeeUpsertService $upsertService
    )
    {
    }

    public function getEmployee(int $id): ManagementEmployeeDetails
    {
        $employee = $this->employeeRepository->getById($id);

        return ManagementEmployeeMapper::mapEmployeeDetails($employee)
            ->setUser(UserMapper::mapUser($employee->getUser()))
            ->setManagement(ManagementMapper::mapManagement($employee->getManagement()));
    }

    public function deleteEmployee(int $id): void
    {
        $employee = $this->employeeRepository->getById($id);
        $this->employeeRepository->removeAndCommit($employee);
    }

    public function createEmployee(ManagementEmployeeRequest $request): IdResponse
    {
        $employee = $this->upsertService->createUpsert((new ManagementEmployee()), $request);
        $this->employeeRepository->saveAndCommit($employee);

        return new IdResponse($employee->getId());
    }

    public function updateEmployee(int $id, ManagementEmployeeRequest $request): void
    {
        $employee = $this->upsertService->updateUpsert($this->employeeRepository->getById($id), $request);
        $this->employeeRepository->saveAndCommit($employee);
    }

}

Сервис, который позволяет выбрать нужный метод по обработке данных (создание или обновление)

ManagementEmployeeUpsertService

<?php
declare(strict_types=1);

namespace App\Feature\Management\Service;

use App\Feature\Management\Entity\ManagementEmployee;
use App\Feature\Management\Model\Employee\ManagementEmployeeRequest;
use App\Feature\Management\Model\Employee\Upsert\AbstractManagementEmployeeUpsert;
use App\Feature\Management\Model\Employee\Upsert\ManagementEmployeeUpsertManager;

readonly class ManagementEmployeeUpsertService
{
    public function __construct(
        private ManagementEmployeeUpsertManager $upsertManager)
    {
    }

    public function createUpsert(ManagementEmployee $employee, ManagementEmployeeRequest $request): ManagementEmployee
    {
        return $this->upsert($this->upsertManager->getCreateUpsert(), $employee, $request);
    }

    public function updateUpsert(ManagementEmployee $employee, ManagementEmployeeRequest $request): ManagementEmployee
    {
        return $this->upsert($this->upsertManager->getUpdateUpsert(), $employee, $request);
    }

    public function upsert(AbstractManagementEmployeeUpsert $upsert, ManagementEmployee $employee, ManagementEmployeeRequest $request): ManagementEmployee
    {
        return $upsert->fill($employee, $request);
    }

}

Сервис-менеджер, который позволяет выбрать конкретный класс по обработке данных

ManagementEmployeeUpsertManager

<?php
declare(strict_types=1);

namespace App\Feature\Management\Model\Employee\Upsert;

readonly class ManagementEmployeeUpsertManager
{
    public function __construct(
        private CreateManagementEmployeeUpsert $createUpsert,
        private UpdateManagementEmployeeUpsert $updateUpsert
    )
    {
    }

    /**
     * @return CreateManagementEmployeeUpsert
     */
    public function getCreateUpsert(): CreateManagementEmployeeUpsert
    {
        return $this->createUpsert;
    }

    /**
     * @return UpdateManagementEmployeeUpsert
     */
    public function getUpdateUpsert(): UpdateManagementEmployeeUpsert
    {
        return $this->updateUpsert;
    }


}

Базовый метод обработки данных, к которому обращаются уже конкретные классы

ManagementEmployeeUpsert

<?php
declare(strict_types=1);

namespace App\Feature\Management\Model\Employee\Upsert;

use App\Feature\Management\Entity\ManagementEmployee;
use App\Feature\Management\Model\Employee\ManagementEmployeeRequest;
use App\Feature\Management\Repository\ManagementEmployeeRepository;
use App\Feature\User\Exception\UserIsAlreadyHiredException;

readonly class ManagementEmployeeUpsert
{
    public function __construct(private ManagementEmployeeRepository $employeeRepository)
    {
    }

    public function isUserNotChanged(ManagementEmployee $employee, ManagementEmployeeRequest $request): bool
    {
        return ($employee->getUser() && $employee->getUser()->getId() === $request->getUserId());
    }

    public function isUserHired(ManagementEmployee $employee, ManagementEmployeeRequest $request): void
    {
        if ($this->employeeRepository->findByUser($employee->getUser())) {
            throw new UserIsAlreadyHiredException();
        }
    }

    public function fill(ManagementEmployee $employee, ManagementEmployeeRequest $request): ManagementEmployee
    {
        $employee->setFired($request->isFired());

        if ($request->getPosition() !== null) {
            $employee->setPosition($request->getPosition());
        }

        if ($request->getParentId() !== null) {
            $employee->setParentId($request->getParentId());
        }

        if ($request->getDescription() !== null) {
            $employee->setDescription($request->getDescription());
        }

        if ($request->getData() !== null) {
            $employee->setData($request->getData());
        }

        return $employee;
    }
}

Абстрактный класс, от которого наследуются конкретные классы по обработке данных

AbstractManagementEmployeeUpsert

<?php
declare(strict_types=1);

namespace App\Feature\Management\Model\Employee\Upsert;

use App\Feature\Management\Entity\ManagementEmployee;
use App\Feature\Management\Model\Employee\ManagementEmployeeRequest;

abstract class AbstractManagementEmployeeUpsert
{
    abstract public function fill(ManagementEmployee $employee, ManagementEmployeeRequest $request): ManagementEmployee;
}

Comments

[sgidlev]

Класс по обработке данных при создании сотрудника

CreateManagementEmployeeUpsert

<?php
declare(strict_types=1);

namespace App\Feature\Management\Model\Employee\Upsert;

use App\Feature\Management\Entity\ManagementEmployee;
use App\Feature\Management\Exception\ManagementEmployeeAlreadyExistsException;
use App\Feature\Management\Model\Employee\ManagementEmployeeRequest;
use App\Feature\Management\Repository\ManagementEmployeeRepository;
use App\Feature\Management\Repository\ManagementRepository;
use App\Feature\User\Repository\UserRepository;

class CreateManagementEmployeeUpsert extends AbstractManagementEmployeeUpsert
{
    public function __construct(
        private readonly ManagementEmployeeRepository $employeeRepository,
        private readonly ManagementEmployeeUpsert     $upsert,
        private readonly ManagementRepository         $managementRepository,
        private readonly UserRepository               $userRepository,
    )
    {
    }

    public function fill(ManagementEmployee $employee, ManagementEmployeeRequest $request): ManagementEmployee
    {
        $management = $this->managementRepository->getManagement($request->getManagementId());
        $user = $this->userRepository->getUser($request->getUserId());

        $employee->setManagement($management);
        $employee->setUser($user);

        $this->checkEmployeeIsExist($employee);
        $this->upsert->isUserHired($employee, $request);

        return $this->upsert->fill($employee, $request);
    }

    public function checkEmployeeIsExist(ManagementEmployee $employee): void
    {
        if ($this->employeeRepository->findByManagementAndUser($employee->getManagement(), $employee->getUser())) {
            throw new ManagementEmployeeAlreadyExistsException();
        }
    }

}

Класс по обработке данных при обновлении сотрудника

UpdateManagementEmployeeUpsert

<?php
declare(strict_types=1);

namespace App\Feature\Management\Model\Employee\Upsert;

use App\Feature\Management\Entity\ManagementEmployee;
use App\Feature\Management\Exception\ManagementEmployeeAlreadyExistsException;
use App\Feature\Management\Model\Employee\ManagementEmployeeRequest;
use App\Feature\Management\Repository\ManagementEmployeeRepository;
use App\Feature\Management\Repository\ManagementRepository;
use App\Feature\User\Repository\UserRepository;

class UpdateManagementEmployeeUpsert extends AbstractManagementEmployeeUpsert
{
    public function __construct(
        private readonly ManagementEmployeeRepository $employeeRepository,
        private readonly ManagementEmployeeUpsert     $upsert,
        private readonly ManagementRepository         $managementRepository,
        private readonly UserRepository               $userRepository,
    )
    {
    }

    public function fill(ManagementEmployee $employee, ManagementEmployeeRequest $request): ManagementEmployee
    {
        $management = $this->managementRepository->getManagement($request->getManagementId());
        $user = $this->userRepository->getUser($request->getUserId());

        $employee->setManagement($management);

        if ($this->upsert->isUserNotChanged($employee, $request)) {
            return $this->upsert->fill($employee, $request);
        }

        $employee->setUser($user);

        $this->isEmployeeExist($employee, $request);
        $this->upsert->isUserHired($employee, $request);

        return $this->upsert->fill($employee, $request);
    }

    public function isEmployeeExist(ManagementEmployee $employee, ManagementEmployeeRequest $request): void
    {
        if ($this->employeeRepository->findByManagementAndUser($employee->getManagement(), $employee->getUser())
        ) {
            throw new ManagementEmployeeAlreadyExistsException();
        }
    }
}

[sgidlev]

UPD: 06.06.23
Реализовал через фабрику

EmployeeCreateInsertion

<?php
declare(strict_types=1);

namespace App\Feature\Management\Factory;

use App\Feature\Management\Entity\ManagementEmployee;
use App\Feature\Management\Exception\ManagementEmployeeAlreadyExistsException;
use App\Feature\Management\Model\Employee\ManagementEmployeeRequest;
use App\Feature\Management\Model\Employee\Upsert\ManagementEmployeeUpsert;
use App\Feature\Management\Repository\ManagementEmployeeRepository;
use App\Feature\Management\Repository\ManagementRepository;
use App\Feature\User\Repository\UserRepository;

class EmployeeCreateInsertion extends EmployeeInsertion
{
    public function __construct(
        private readonly ManagementEmployeeRepository $employeeRepository,
        private readonly ManagementEmployeeUpsert     $upsert,
        private readonly ManagementRepository         $managementRepository,
        private readonly UserRepository               $userRepository,
    )
    {
    }

    public function insert(ManagementEmployee $employee, ManagementEmployeeRequest $request): ManagementEmployee
    {
        $management = $this->managementRepository->getManagement($request->getManagementId());
        $user = $this->userRepository->getUser($request->getUserId());

        $employee->setManagement($management);
        $employee->setUser($user);

        $this->checkEmployeeIsExist($employee);
        $this->upsert->isUserHired($employee, $request);

        return $this->upsert->fill($employee, $request);
    }

    public function checkEmployeeIsExist(ManagementEmployee $employee): void
    {
        if ($this->employeeRepository->findByManagementAndUser($employee->getManagement(), $employee->getUser())) {
            throw new ManagementEmployeeAlreadyExistsException();
        }
    }
}

EmployeeInsertion

<?php
declare(strict_types=1);

namespace App\Feature\Management\Factory;

use App\Feature\Management\Entity\ManagementEmployee;
use App\Feature\Management\Model\Employee\ManagementEmployeeRequest;

abstract class EmployeeInsertion
{
    abstract public function insert(ManagementEmployee $employee, ManagementEmployeeRequest $request): ManagementEmployee;
}

EmployeeInsertionFactory

<?php
declare(strict_types=1);

namespace App\Feature\Management\Factory;

use App\Application\Enum\InsertType;

interface EmployeeInsertionFactory
{
    public function makeInsertion(InsertType $insertType): EmployeeInsertion;
}

EmployeeInsertionFactoryImpl

<?php
declare(strict_types=1);

namespace App\Feature\Management\Factory;

use App\Application\Enum\InsertType;
use App\Infrastructure\Exception\InvalidInsertTypeException;

readonly class EmployeeInsertionFactoryImpl implements EmployeeInsertionFactory
{
    public function __construct(
        private EmployeeCreateInsertion $createInsertion,
        private EmployeeUpdateInsertion $updateInsertion
    )
    {

    }

    public function makeInsertion(InsertType $insertType): EmployeeInsertion
    {
        return match ($insertType) {
            InsertType::CREATE => $this->createInsertion,
            InsertType::UPDATE => $this->updateInsertion,
            default => throw new InvalidInsertTypeException(),
        };
    }
}

EmployeeUpdateInsertion

<?php
declare(strict_types=1);

namespace App\Feature\Management\Factory;

use App\Feature\Management\Entity\ManagementEmployee;
use App\Feature\Management\Exception\ManagementEmployeeAlreadyExistsException;
use App\Feature\Management\Model\Employee\ManagementEmployeeRequest;
use App\Feature\Management\Model\Employee\Upsert\ManagementEmployeeUpsert;
use App\Feature\Management\Repository\ManagementEmployeeRepository;
use App\Feature\Management\Repository\ManagementRepository;
use App\Feature\User\Repository\UserRepository;

class EmployeeUpdateInsertion extends EmployeeInsertion
{
    public function __construct(
        private readonly ManagementEmployeeRepository $employeeRepository,
        private readonly ManagementEmployeeUpsert     $upsert,
        private readonly ManagementRepository         $managementRepository,
        private readonly UserRepository               $userRepository,
    )
    {
    }

    public function insert(ManagementEmployee $employee, ManagementEmployeeRequest $request): ManagementEmployee
    {
        $management = $this->managementRepository->getManagement($request->getManagementId());
        $user = $this->userRepository->getUser($request->getUserId());

        $employee->setManagement($management);

        if ($this->upsert->isUserNotChanged($employee, $request)) {
            return $this->upsert->fill($employee, $request);
        }

        $employee->setUser($user);

        $this->isEmployeeExist($employee, $request);
        $this->upsert->isUserHired($employee, $request);

        return $this->upsert->fill($employee, $request);
    }

    public function isEmployeeExist(ManagementEmployee $employee, ManagementEmployeeRequest $request): void
    {
        if ($this->employeeRepository->findByManagementAndUser($employee->getManagement(), $employee->getUser())
        ) {
            throw new ManagementEmployeeAlreadyExistsException();
        }
    }
}

ManagementEmployeeService

<?php
declare(strict_types=1);

namespace App\Feature\Management\Service;

use App\Application\Enum\InsertType;
use App\Application\Model\Response\IdResponse;
use App\Feature\Management\Entity\ManagementEmployee;
use App\Feature\Management\Factory\EmployeeInsertionFactoryImpl;
use App\Feature\Management\Mapper\ManagementEmployeeMapper;
use App\Feature\Management\Mapper\ManagementMapper;
use App\Feature\Management\Model\Employee\ManagementEmployeeDetails;
use App\Feature\Management\Model\Employee\ManagementEmployeeRequest;
use App\Feature\Management\Repository\ManagementEmployeeRepository;
use App\Feature\User\Mapper\UserMapper;

readonly class ManagementEmployeeService
{
    public function __construct(
        private ManagementEmployeeRepository $employeeRepository,
        private EmployeeInsertionFactoryImpl $factory
    )
    {
    }

    public function getEmployee(int $id): ManagementEmployeeDetails
    {
        $employee = $this->employeeRepository->getById($id);

        return ManagementEmployeeMapper::mapEmployeeDetails($employee)
            ->setUser(UserMapper::mapUser($employee->getUser()))
            ->setManagement(ManagementMapper::mapManagement($employee->getManagement()));
    }

    public function deleteEmployee(int $id): void
    {
        $employee = $this->employeeRepository->getById($id);
        $this->employeeRepository->removeAndCommit($employee);
    }

    public function createEmployee(ManagementEmployeeRequest $request): IdResponse
    {
        $employee = $this->factory
            ->makeInsertion(InsertType::CREATE)
            ->insert(new ManagementEmployee(), $request);

        $this->employeeRepository->saveAndCommit($employee);

        return new IdResponse($employee->getId());
    }

    public function updateEmployee(int $id, ManagementEmployeeRequest $request): void
    {
        $employee = $this->factory
            ->makeInsertion(InsertType::UPDATE)
            ->insert($this->employeeRepository->getById($id), $request);

        $this->employeeRepository->saveAndCommit($employee);
    }

}

InsertType

<?php
declare(strict_types=1);

namespace App\Application\Enum;

enum InsertType: int implements BaseEnumInterface
{
    use BaseEnumTrait;

    case CREATE = 1; // on create request
    case UPDATE = 2; // on update request
}

Answer 3

[sl0]

Задача контроллера решить, что именно нужно делать: создать юзера или обновить, и исходя из этого уже дернуть нужный метод в сервисе. Дублирующийся код в сервисе можно вынести в отдельный метод, к которому будут обращаться и create, и update - проблем с этим не вижу.