New Transit::attach() method to attach domain to the current context#16
New Transit::attach() method to attach domain to the current context#16ElGigi wants to merge 2 commits intoatlasphp:0.xfrom
Conversation
|
@pmjones Related to my question in atlasphp/Atlas.Orm#98 (comment) |
src/Transit.php
Outdated
| /** | ||
| * Attach a domain object to plan. | ||
| * | ||
| * @param \Atlas\Transit\object $domain |
There was a problem hiding this comment.
This class doesn't exists \Atlas\Transit\object. Use object instead.
| * @return \Atlas\Transit\Transit | ||
| * @throws \Atlas\Transit\Exception | ||
| */ | ||
| public function attach(object $domain): Transit |
There was a problem hiding this comment.
I see a problem here: which method should the end user use? store or attach? This results in an unclear API.
There was a problem hiding this comment.
Like Doctrine; it's needed to "attach" ("merge" with Doctrine) the entity to Transit before modification.
If the entity is modified and stored, she is already present and an update is done.
There was a problem hiding this comment.
If you have an another suggestion of method name, i'm ready :)
There was a problem hiding this comment.
Doctrine no longer works as an example, because merge and detach are deprecated and will be removed in version 3. 😉
https://github.com/doctrine/orm/blob/master/UPGRADE.md#bc-break-removed-entitymanagermerge-and-entitymanagerdetach-methods
doctrine/orm#1577 (comment)
There was a problem hiding this comment.
Yes i see that 👍 .
But how do for the current problematic?
It's the job of a framework? or need to integrate a concept into Transit?
There was a problem hiding this comment.
See also at Doctrine:
It is a good idea to avoid storing entities in serialized formats such as
$_SESSION: instead, store the entity identifiers or raw data.
…and this is the path you should go.
There was a problem hiding this comment.
It's not too similar to Transit::select(string $domainClass, array $whereEquals = []) method; with second parameter who accept a scalar or array argument?
There was a problem hiding this comment.
It's not too similar to…
You are on the right track. 😉
Don't serialize your domain objects, instead:
- store the entity identifiers or raw data (in session, etc.)
- use the existing
selectmethod to get your domain object
…and the entire pull request is not needed.
New Transit::attach() method who permit to add a domain to the handler storage.
It's permit to update a domain from another context, without fetch domain again before ; like domain getted from session.
Similar with Doctrine: https://www.doctrine-project.org/projects/doctrine-orm/en/2.6/cookbook/entities-in-session.html
Issue: #15