-
Notifications
You must be signed in to change notification settings - Fork 3
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Move ImmutableRecord to own repository and use ImmutableObject trait #106
Comments
Note: I wrote the implementation in the gist just today as a proof of concept. I didn't even try to use is anywhere yet except the testcase. I might be able to help with some PRs later but it depends on if I do end up trying to use the event-machine or not. With this plan I will reconsider. Feel free to use my code from the gist under the terms of the WTFPL license. 😛 |
@enumag |
My thoughts exactly. I'm no stranger to using wild reflection tricks like this. They need to be implemented and used carefully but can help a lot. I do intend to use this trait in my project. For now just with annotated properties which of course loses some type-safety but it's not much of an issue thanks to PHPStan and possible migration to PHP 7.4 properties next year. I already implemented one simple aggregate using that trait and some EventMachine inspiration and it looks pretty good in my opinion. No dependencies on anything outside of the domain save for that trait. And even the trait could be removed in the future if PHP ever gets support for property accessors. <?php declare(strict_types = 1);
namespace App\Context\RealtyRegistration\DomainModel\ZipCode;
use App\Context\RealtyRegistration\DomainModel\ZipCode\Command\RegisterZipCodeCommand;
use App\Context\RealtyRegistration\DomainModel\ZipCode\Command\UnlistZipCodeCommand;
use App\Context\RealtyRegistration\DomainModel\ZipCode\Event\ZipCodeRegistered;
use App\Context\RealtyRegistration\DomainModel\ZipCode\Event\ZipCodeUnlisted;
use Generator;
final class ZipCode
{
public static function register(RegisterZipCodeCommand $command): Generator
{
yield ZipCodeRegistered::create(
function (ZipCodeRegistered $event) use ($command): void {
$event->zipCodeId = $command->zipCodeId;
$event->zipCodeData = $command->zipCodeData;
}
);
}
public static function unlist(ZipCodeState $state, UnlistZipCodeCommand $command): Generator
{
yield ZipCodeUnlisted::create(
function (ZipCodeUnlisted $event) use ($state): void {
$event->zipCodeId = $state->zipCodeId;
}
);
}
public static function applyZipCodeRegistered(ZipCodeRegistered $event): ZipCodeState
{
return ZipCodeState::create(
function (ZipCodeState $state) use ($event): void {
$state->zipCodeId = $event->zipCodeId;
$state->zipCodeData = $event->zipCodeData;
}
);
}
public static function applyZipCodeUnlisted(ZipCodeState $state, ZipCodeUnlisted $event): ZipCodeState
{
return $state->cloneUsing(
function (ZipCodeState $state) use ($event): void {
$state->unlisted = true;
}
);
}
} |
Thinking about it I could simplify it further by sending a cloned mutable version of the state into the public static function applyZipCodeUnlisted(ZipCodeState $state, ZipCodeUnlisted $event): ZipCodeState
{
$state->unlisted = true;
} |
I would not pass a mutable version of the object into the function. Are the properties of An immutable collection of apartment attributes replaces the old one. That's the reason why I want to combine your $address = Address::fromRecordData([
'street' => Street::fromString('Main Road'),
'city' => City::fromString('Somewhere'),
]);
echo $address->street; //Main Road
$changedAddress = $address->with([
'street' => 'Another Street'
]);
echo $address->street; //Main Road
echo $changedAddress->street; //Another Street
echo $changedAddress->city; //Somewhere |
Yeah I guess you're right. I still don't like your usage of arrays though since IDE won't be able to suggest the keys. So I'll stick to my Closure initializers for that reason. Anyway another thing I was thinking about was to move the apply methods from the Aggregate class to the State class. Currently the aggregate implements both the generators to yield new events and the apply methods to create new State which kinda violates SRP in my opinion. The main logic is always in the generators which need to be in the aggregate of course. But the apply methods are straightforward copy-pasting of event data to the state so I feel like they belong either to the State class or into some other StateUpdater class separate from the aggregate logic. |
I use constants to get around both: typos and missing IDE support You can even express key relations like shown above: State::BUILDING_ID is identical to Payload::BUILDING_ID. I've a set of PHPStorm live + class templates to generate those classes, constants and properties quickly. Needs a bit practice but then you're very productive and typos belong to the past.
You can definitely do that. For me the aggregate class turned into the last part of the naemspace, because when I work with Event Machine I always use the functional approach so those aggregate functions are just grouped in class, but each function is independent of the others. SRP is not violated because we don't talk about an object doing different things. We talk about public static functions of a class and each function does only one thing: either handle a command or apply an event ;). The functions need to be stateless and side-effect free, though. https://proophsoftware.github.io/event-machine/api/aggregates.html#3-3-1 |
@enumag uses a neat trick to unset public properties of an object, see this gist
This way magic __get and __set methods of the object are called instead and they can enforce immutability.
This mechanism can even be deactivated in production for performance reasons and it can be combined with the ImmutableRecord logic used for immutable data types in Event Machine powered projects.
To be able to use immutable data types without installing Event Machine, the package should be moved to its own repo.
The text was updated successfully, but these errors were encountered: