Skip to content
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

Add support for array of objects as reference #141

Open
wants to merge 22 commits into
base: 1.5.x
Choose a base branch
from

Conversation

flip111
Copy link
Contributor

@flip111 flip111 commented Apr 22, 2014

Could not start test from working test code, see #142

Why is this useful? Consider model relations:
A OneToMany B OneToMany C (a tree)
When creating the entities for B, it's easy to grab that one A. However when creating fixtures for C there are now multiple B's, they clearly belong together (are part of the same A), but they can't be grouped. The only alternative is to do addReference('B1', $Bs[0]); addReference('B2', $Bs[1]); etc

Dirk Luijk and others added 20 commits August 13, 2013 17:20
PhpStorm shows an error. I think it's because ObjectManager was already imported.
addReference() actually throws exception
Explicit modifier scope for function load
Add explicit visibility for function load
Syntax highlighting on README.md
Add explicit visibility following PSR-2
Add explicit visibility for method getOrder
@@ -98,7 +98,7 @@ protected function getIdentifier($reference, $uow)
* @param string $name
* @param object $reference
*/
public function setReference($name, $reference)
public function setReference($name, object $reference)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

object is not a valid typehint in PHP

@flip111
Copy link
Contributor Author

flip111 commented Apr 23, 2014

setting multiple references for a single name does not make sense

@stof please motivate your answer, i shall edit the first post to motivate my PR

return $this->_loadReference($name, $references);
}

$return = [];
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

invalid PHP 5.3 code

@stof
Copy link
Member

stof commented Apr 23, 2014

@flip111 the goal of the ReferenceRepository is to keep a reference to a managed entity to share it between fixtures ("managed" being the reason why we need some logic for it). Having 2 different meaning for the reference seems weird to me.

Thus, even the naming is confusing: setting a reference to an arrya is done with setReferences in your code, which is telling it is registering many references, not one. And then, you still get it with getReference. This is a clear way to see that even for you, the meaning of a reference is not clear anymore

@flip111
Copy link
Contributor Author

flip111 commented Apr 23, 2014

@stof i agree with what you are saying. However i was too afraid to adjust more code then i already have and come up with new names for "reference". Anyway if you or anyone else agrees that the use case is there and validates a change then we can discuss a possible solution to this problem.

Right now it's up to the programmer to remember if a single reference is a reference to an object or an array of objects. But actually i think the setReference method should detect if the $references is an array and then the setReferences method can be removed.

By the way, didn't know this lib had to be compatible with php 5.3

Base automatically changed from master to 1.5.x January 23, 2021 10:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants