RFC: Always enforce updating inverse side for Comment

[bocharsky-bw]

It's an edge case, but what do you think about always updating inverse side in adder/remover methods?

For example, currently this code won't update inverse side properly:

$comment = new Comment();
$post->getComments()->add($comment);
// but then if we will call adder method
$post->addComment($comment); // Comment::$post is still equals null here due to the failed `!contains()` check in it

[voronkovich]

@bocharsky-bw, this code checks that a comment belongs to a post. See http://www.doctrine-project.org/api/common/2.3/class-Doctrine.Common.Collections.Collection.html#_removeElement

[bocharsky-bw]

Yes, you're correct. But here's another example to show that this code has a bit unexpected behavior:

$comment = new Comment();
$post->addComment($comment); // Comment::$post === $post object

$post->getComments()->removeElement($comment);
$post->removeComment($comment); // Comment::$post is still equal to $post object here due to the failed check in this method

[javiereguiluz]

Thanks @bocharsky-bw.

[stof]

My suggestion would be to stop using bi-directional relations instead, whenever we can (i.e. following the official Doctrine recommendation)

[javiereguiluz]

Did this recommendation changed recently? Do you have some links to docs or GitHub issues/PRs? Thanks!

[voronkovich]

Doctrine documentation says:

If you want to make sure that your collections are perfectly encapsulated you should not return them from a getCollectionName() method directly, but call $collection->toArray(). This way a client programmer for the entity cannot circumvent the logic you implement on your entity for association management.

See http://docs.doctrine-project.org/projects/doctrine-orm/en/latest/reference/working-with-associations.html#association-management-methods

I would suggest to use toArray method.

[voronkovich]

@javiereguiluz, I think @stof means doctrine best practices http://docs.doctrine-project.org/projects/doctrine-orm/en/latest/reference/best-practices.html#constrain-relationships-as-much-as-possible

[bocharsky-bw]

If we can't or don't want to get rid of bi-directional relations as @stof suggested, then @voronkovich idea makes sense I think. Looks like with bi-directional relations there's no perfect solution.