Move ImmutableRecord to own repository and use ImmutableObject trait

[codeliner]

Move ImmutableRecord to own repository and use ImmutableObject trait

[enumag]

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;
			}
		);
	}
}

[codeliner]

@enumag unset($this->{$property}) is brilliant. Of course I thought about magic __get and __set but this hack did not come to mind. Also thought about a way to disable the mechanism in production and with your trait it should be doable. It looks simple enough to me that I don't see problems with this approach. For example we use Reflection logic a lot in the ImmutableRecordLogic trait. Looks wild, works great ;)

[enumag]

Thinking about it I could simplify it further by sending a cloned mutable version of the state into the apply methods and freezing it right after. WDYT? In case of the code above the last method would be changed into this:

	public static function applyZipCodeUnlisted(ZipCodeState $state, ZipCodeUnlisted $event): ZipCodeState
	{
		$state->unlisted = true;
	}

[codeliner]

I would not pass a mutable version of the object into the function. Are the properties of ZipCodeState mutable or immutable? Mixing both is confusing and could lead to new sources of errors. Immutable data types are great because they avoid certain types of bugs.

See here: https://github.com/proophsoftware/fee-office/blob/master/src/RealtyRegistration/src/Model/Apartment.php#L78

An immutable collection of apartment attributes replaces the old one.

That's the reason why I want to combine your ImmutableObject trait with our ImmutableRecordLogic trait. I still want to use the immutable API:

$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

[enumag]

I would not pass a mutable version of the object into the function. Are the properties of ZipCodeState mutable or immutable? Mixing both is confusing and could lead to new sources of errors

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.

[codeliner]

I still don't like your usage of arrays though since IDE won't be able to suggest the keys.

I use constants to get around both: typos and missing IDE support
See here for example: https://github.com/proophsoftware/fee-office/blob/master/src/RealtyRegistration/src/Model/Building/State.php#L14

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.

move the apply methods from the Aggregate class to the State class

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