Skip to content

hw14 - Domain Driven Design#228

Open
ivanzn55 wants to merge 1 commit into
IZolotarev/hw1from
IZolotarev/hw14
Open

hw14 - Domain Driven Design#228
ivanzn55 wants to merge 1 commit into
IZolotarev/hw1from
IZolotarev/hw14

Conversation

@ivanzn55

Copy link
Copy Markdown

No description provided.

private \DateTimeInterface $createdAt;
private \DateTimeInterface $updatedAt;

public function __construct(

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

В конструкторе тоже неплохо бы проверять данные.. можно ведь и -14 в quantity передавать


}

private function validate(): void

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. метод нигде не используется
  2. зачем он нужен? модель всегда должна быть в валидном состоянии, как после создания, так и после любой мутации

public function findById(string $id): ?Cart;
public function findByUserId(int $userId): ?Cart;
public function findByGuestToken(string $guestToken): ?Cart;
public function save(Cart $cart): Cart;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

зачем тут возвращаемое значение? вы уже объект передаете в функцию

use App\Domain\Cart\Repositories\CartRepositoryInterface;


class CartService

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Сервис как-буд-то не нужен. У вас есть аналогичный сервис в слое Application, который сейчас служит прослойкой к этому сервису. вот вся эта логика которая здесь есть больше подходит к слою Application. репозитории я тоже отношу к слою application, так как репозиторий - это инфраструктурный сервис для ввнешнего хранилища. Но вы в интернете и литературе вы встертите его в доменном слое тоже. Все что требует ккую-то внешнюю инфраструктуру - я считаю слоем Application. Так вот в сервисе у вас логика, которая связывает внешнюю инфраструктуру и объекты доменной модели. Значит и сервис принадлежит уровню Application.

return $this->cartRepository->save($cart);
}

public function mergeCarts(Cart $sourceCart, Cart $targetCart): Cart

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

этот метод единственное исключение из правила выше.. Потому что здесь у вас операция бизнесовая по слиянию двух агрегатов. Если у вас бизнес логика с несколькими агрегатами - тут как раз доменный сервис и может понадобиться. Однако в доменном сервисе вы не должны работать с инфраструктурой (сохранение в репозиторий) - это уже на слое выше в Application делается. Тут только чистая бизнес логика слияния корзин.

$this->value = $value;
}

public function getValue(): int

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

субъективно рекомендую нейминг toInt() toString() и т.п.

return $this->order;
}

public function toArray(): array

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

$this->validate();
}

private function validate(): void

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Если такой метод поялвяется, значет вы хотите убрать где-то дублированные проверки.. а значит есть хорошие кандидаты для ValueObject. Например, если сделать VO для Email


public function cancel(): void
{
if (!$this->canBeCancelled()) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Зачем отдельнй метод?

}

public static function createFromCart(
Cart $cart,

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Тут у вас жетская связь на Cart идет. Такой фабричный метод лучше вынести в отдельный класс-фабрику, а не в самой модели это делать.

return $model ? $this->toEntity($model) : null;
}

public function save(Cart $cart): Cart

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ага.. понятно почему у вас возвращается объект Cart. Вы по сути не сохраняете передаваемый объект, а создаете и возвращаете новый. Честно говоря не совсем ожидаемое поведение, я бы так не делал. Зачем? если в процессе сохранения что-то ожидается что изменится, ну обновите в текущем объекте..

$result = $this->orderAppService->getOrdersPaginated($page, $perPage, $criteria);

return response()->json([
'data' => array_map(fn($order) => $order->toArray(), $result['data']),

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Если строго, то объекты доменной модели не должны выползать за пределы слоя Application. Тут вы в слое Presenstion/Interface напрямую работаете с ними. если для себя - ну ок. Если суровый enterprise, то вместо объектов доменной модели сервисы слоя Application должны возвращать собственные объекты DTO, в которые данные доменных моделий будут маппиться

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants