prooph / proophessor-do-symfony

Symfony version of proophessor-do CQRS + Event Sourcing example app
http://getprooph.org/
Other
170 stars 52 forks source link

Autdated docs and samples #31

Closed BonBonSlick closed 5 years ago

BonBonSlick commented 6 years ago

http://docs.getprooph.org/tutorial/event_sourcing_basics.html and here you pass VO to event. https://github.com/prooph/proophessor-do-symfony/blob/0d4140c6ac75943d5ff447dadc664b6a7300b4a1/src/ProophessorDo/Model/User/User.php

prolic commented 6 years ago

I don't know what you are looking for

BonBonSlick commented 6 years ago

@prolic

//Start new aggregate lifecycle by creating an "empty" instance
        $self = new self();

        //Record the very first domain event of the new aggregate
        //Note: we don't pass the value objects directly to the event but use their
        //primitive counterparts. This makes it much easier to work with the events later
        //and we don't need complex serializers when storing events.
        $self->recordThat(ShoppingSessionStarted::occur($basketId->toString(), [
            'shopping_session' => $shoppingSession->toString()
        ]));

    public static function registerWithData(
        UserId $userId,
        UserName $name,
        EmailAddress $emailAddress
    ): User {
        $self = new self();
        $self->recordThat(UserWasRegistered::withData($userId, $name, $emailAddress));
        return $self;
    }

And confusing


 $self->recordThat(UserWasRegistered::occur($entityId->value(), [
            'name' => userId,
          ....
        ]));
// or
        $self->recordThat(UserWasRegistered::withData($userId, $name,...));
prolic commented 6 years ago

occur:

https://github.com/prooph/event-sourcing/blob/master/src/AggregateChanged.php#L28-L30

withData:

something you implement yourself

BonBonSlick commented 6 years ago

@prolic yeah. Maybe add in documentation the capability, difference and the best way to do it? Which way you would recommend and why?

BonBonSlick commented 6 years ago

@prolic also noticed that 2 projects almost identical, to avoid double job and support of these two, maybe create only for symfony something specific? Like config files, yaml, xml, etc. https://github.com/prooph/proophessor-do-symfony/blob/master/src/ProophessorDo/Model/User/User.php https://github.com/prooph/proophessor-do/blob/master/src/Model/User/User.php

BonBonSlick commented 6 years ago

@also, here in examples comparison goes with get_class($this) === get_class($other) which indicates class A === class A and in docs http://docs.getprooph.org/tutorial/event_sourcing_basics.html

 public function equals($other): bool
    {
        if(!$other instanceof self) {
            return false;
        }

        return $this->shoppingSession === $other->shoppingSession;
    }

which indicates class A == class A or inherited from B or C and other extended classes, will return true. Example, I can compare UserId extends AbstracIt, in case UserId instance AbstractId returns true.

 $user = UserId::next();
     $user = UserId::next();
    $test = AbstractId::next();
        \dump($user instanceof  $test); // true
        die;

So the question why different approaches are used in samples and docs?

BonBonSlick commented 6 years ago

also, empty method https://github.com/prooph/proophessor-do/blob/master/src/Model/User/User.php#L90 and https://github.com/prooph/proophessor-do-symfony/blob/master/src/ProophessorDo/Model/User/User.php#L90 because was copy pasted

BonBonSlick commented 6 years ago

I have created demo user class

$self->recordThat(UserCreated::withData($entityId)); says

Expected \Prooph\EventSourcing\AggregateChanged, got \App\Event\UserCreated 
Invocation parameter types are not compatible with declared.

while

use Prooph\EventSourcing\AggregateChanged;
final class UserCreated extends AggregateChanged

Why warning then?

prolic commented 6 years ago

This Invocation parameter types are not compatible with declared. is a PhpStorm warning only, right? If so, forget about this one.

Second, listen close, this is VERY IMPORTANT: You should NOT couple your domain model to any framework or library, including prooph/event-sourcing. Use it as a reference implementation to roll your own. You can also find this in the event sourcing docs: https://github.com/prooph/event-sourcing/blob/master/docs/repositories.md#event-sourced-aggregates

That's exactly the reason why prooph/event-sourcing gets completely deprecated next year. People ignore this very important fact and just install prooph/event-sourcing into their application.

Third: I'm sure there are things in the docs to improve, please don't expect anyone to improve the docs whenever someone finds something that may need improvement. We do this in our spare time. We do this for free. We are not paid doing all this. Please submit a PR for improvements and help us out. We can't do everything.

BonBonSlick commented 6 years ago

@prolic ok, thanks