auraphp / Aura.Payload

A Domain Payload implementation.
MIT License
55 stars 18 forks source link

Added Immutable Payload #13

Closed damianopetrungaro closed 7 years ago

jakejohns commented 7 years ago

IMO, this would be better served with a new interface. I've purposed separating read/write here auraphp/Aura.Payload_Interface#4

Once that's done, I'd think an ImutablePayload would implement the ReadablePayload and define "with style" methods (withStatus) rather than "setters" as the semantics are confusing.

damianopetrungaro commented 7 years ago

Totally agree about the "with" instead of "set".

Also the read/write implementation seems to be coherent.

Let me know when auraphp/Aura.Payload_Interface#4 is accepted.

pmjones commented 7 years ago

Hey @damianopetrungaro --

I don't think that deepClone() as implemented https://github.com/auraphp/Aura.Payload/pull/13/files#diff-8ad8eea98946c901b70d906fa800079dR225 is sufficient to enforce immutability of the objects carried by the payload. Merely cloning the object in the Payload is insufficient; the object being cloned must also "deep clone" itself, which cannot be guaranteed. Then there's resources and references to contend with. Cf. http://paul-m-jones.com/archives/6400

The amount of work needed to enforce immutability "all the way down" is rather extensive.

damianopetrungaro commented 7 years ago

I may implement logic for check if the entry in the payload is fully immutable, if is't an exception will be thrown.

It's all about how we want (if we want) to implement it. In my free time a can write some code for this case. Let me know if is wanted this feature. (It's also a good exercise make a fully immutable object, so for me it's ok)

pmjones commented 7 years ago

I may implement logic for check if the entry in the payload is fully immutable

I am curious to see how that would work.

pmjones commented 7 years ago

@damianopetrungaro Oh, and:

Let me know if is wanted this feature.

Well, it's wanted -- if it can actually be done. I don't think it can, with PHP as it exists now, but I've been wrong before. ;-)

damianopetrungaro commented 7 years ago

At the moment PHP doesn't any any way for implement a totally immutable object, but using reflection something could(?) be done.

I have never deepened the argument, but it may be worth some experiments 😄 Give me couple days for see a little bit more the argument 😄

pmjones commented 7 years ago

Give me couple days for see a little bit more the argument

Good enough!

damianopetrungaro commented 7 years ago

@pmjones After thinking a little bit about it i have found those rules that may preserve an immutable object.

Those are the points that could be used for immutable object/payload. What do you think about it?

pmjones commented 7 years ago

if is an object it should be immutable (or at least must be cloneable)

My understanding is that all the properties of that object need to remain immutable as well; so, not just cloneable, but cloneable in such a way that all of its objects are cloned. And all the objects in its arrays are cloned. And all resources held by the object are read-only. (You begin to see the trouble.)

damianopetrungaro commented 7 years ago

@pmjones And the answer to this stuff may be:

  • on 'getter' if return type is an object should return a cloned instance for preserve immutability, because atm php can't guarantee that the object retrieved is immutable.

So the retrieved object (and its own properties) are cloned. This is the only (but really, really ugly) way i see for a real immutable in php atm.

pmjones commented 7 years ago

Even the "clone on get" is insufficient, because the object being cloned may contain objects that are not cloned, which means the original and the clone now use the same underlying object. So, not immutable.

This is the only (but really, really ugly) way i see for a real immutable in php atm.

It is ugly, and resource-intensive. Which is why I think it's not worth the effort for a generic immutable Payload library -- at least, not with PHP in its current state.

Now, for your own payload class, you can ensure immutability by making your domain objects immutable, and then checking those objects in your own payload class (using an interface or whatever). Much simpler, though not generic.

damianopetrungaro commented 7 years ago

Another approach may be: Let stuff be as they are now, and write in the documentation that immutable payload MUST be implemented by the userland, linking the issue and your blogpost. Because in my domain i can use immutable object and type hint the payload for only immutable object.

pmjones commented 7 years ago

Let stuff be as they are now, and write in the documentation that immutable payload MUST be implemented by the userland, linking the issue and your blogpost.

You could do that now and only pass immutable objects. ;-)

damianopetrungaro commented 7 years ago

Now, for your own payload class, you can ensure immutability by making your domain objects immutable, and then checking those objects in your own payload class (using an interface or whatever). Much simpler, though not generic.

You anticipate me 😄 , that's the better approach.