dddshelf / ddd-in-php-book-issues

Leave your comments, improvements or book mistakes as an issue! Thanks ❤️
https://leanpub.com/ddd-in-php
28 stars 2 forks source link

A few issues I (Vladas) noticed #65

Closed vladas closed 7 years ago

vladas commented 8 years ago

Paragraph 8.6

$order = ...
$orderLine = new OrderLine('Domain-Driven Design in PHP', 24.99);
$order->addOrderLine($order)

should be:

$order = ...
$orderLine = new OrderLine('Domain-Driven Design in PHP', 24.99);
$order->addOrderLine($orderLine)

Typo?

The following sentence probably should be reviewed:

Instead, our advice is to map your relations in your ORM just is two Entities from an Aggregate.

I assume it was supposed to be "in two Entities".

Undefined variable

$wishId in the following example is undefined.

class User
{
    // ...
    /**
    * @return void
    */
    public function makeWish($address, $content)
    {
        if (count($this->wishes) >= 3) {
            throw new MaxNumberOfWishesExceededException();
        }
        $this->wishes[] = new Wish(
            $wishId,
            $this->id(),
            $address,
            $content
        );
    }
    // ...
}

BaseUserFactory => UserFactory?

In the following code snippet I assume UserFactory should be used instead of BaseUserFactory as it's defined.

class DoctrineUserFactory implements BaseUserFactory
{
    public function build(UserId $userId, $email, $password)
    {
        return new DoctrineUser($userId, $email, $password);
    }
}

Typo sort => short:

throw new \DomainException('Body is too sort');

DataTransformer state in service

I'm quite sure that it was mentioned in this book, that services shouldn't contain any state. However, in the following example service contains state. Why is state in this example allowed?

class SignUpUserService
{
    private $userRepository;
    private $userDataTransformer;
    public function __construct(
        UserRepository $userRepository,
        UserDataTransformer $userDataTransformer
    )
    {
        $this->userRepository = $userRepository;
        $this->userDataTransformer = $userDataTransformer;
    }
    public function execute(SignUpUserRequest $request)
    {
        // ...
        $user = // ...
        $this->userDataTransformer->write($user);
    }
    /**
    * @return UserDataTransformer
    */
    public function userDataTransformer()
    {
        return $this->userDataTransformer;
    }
}

Hi guys,

I enjoyed the book a lot. Thanks. Here are somethings I noticed.

DomainEventPublisher singelton

What is the justification for having DomainEventPublisher as a singleton? Doesn't it hide dependency?

DomainEventPublisher::instance()->publish(
    new PostPublished($postId)
);

It's appearant that Unit tests suffer from singelton problems as well when you have to unsubscribe:

class SignUpUserServiceTest extends \PHPUnit_Framework_TestCase
{
    // ...
    /**
    * @test
    */
    public function itShouldPublishUserRegisteredEvent()
    {
        $subscriber = new SpySubscriber();
        $id = DomainEventPublisher::instance()->subscribe($subscriber);

        $user = $this->executeSignUp();
        $userId = $user->id();

        DomainEventPublisher::instance()->unsubscribe($id);
        $this->assertUserRegisteredEventPublished($subscriber, $userId);
}

Regards, Vladas Dirzys

carlosbuenosvinos commented 7 years ago

All fixes done. Thanks! You are now in the Acknowledgements.

Related to your questions: 1.- You're right. If you don't like it, you can just return the result of the transformation. With that way, both (application service and data transformer) would be stateless. However, it's not a big deal this approach ;)

2.- You have multiple options: Singleton: It works well in most all scenarios Passing the event dispatcher all around: Too gossip. Do you imagine all your Entities receiving the Event Dispatcher as parameter or constructor? Nop. Aggregates collecting all events and then getter: for Aggregates it works in general. However, sometimes is not easy to know what Aggregates where modified and get all the events to publish them. It's also more difficult to fire events from other places such as Domain Services or even Application Services (you should avoid that, but sometimes can work for you legacy projects).