[GH-34] Instead of NULL, unitialize typed properties without default. #35
Conversation
beberlei
force-pushed
the
GH-34-SetValueUnitialized
branch
from
8b195cf
to
cd4811c
Compare
| if ($value === null) { | ||
| $propertyName = $this->getName(); | ||
|
|
||
| $unsetter = function () use ($propertyName) { | ||
| unset($this->$propertyName); | ||
| }; | ||
| $unsetter = $unsetter->bindTo($object, $this->getDeclaringClass()->getName()); | ||
| $unsetter(); | ||
|
|
||
| return; | ||
| } | ||
|
|
||
| parent::setValue($object, $value); |
There was a problem hiding this comment.
Is it worth to keep a property with the original closure? Doing that is safe since Closure#bindTo() is a immutable operation. It would avoid creating maaaaany unnecessary closures in runtime but we should benchmark it to see the impact (memory/processing time.)
That would simplify this quite a lot, considering the unsetter is created at construction time:
| if ($value === null) { | |
| $propertyName = $this->getName(); | |
| $unsetter = function () use ($propertyName) { | |
| unset($this->$propertyName); | |
| }; | |
| $unsetter = $unsetter->bindTo($object, $this->getDeclaringClass()->getName()); | |
| $unsetter(); | |
| return; | |
| } | |
| parent::setValue($object, $value); | |
| if ($value === null) { | |
| $this->unsetter->bindTo($object, $this->getDeclaringClass()->getName())(); | |
| return; | |
| } | |
| parent::setValue($object, $value); |
There was a problem hiding this comment.
We could also save the result of $this->getDeclaringClass()->getName() to reduce call count to these methods, btw.
There was a problem hiding this comment.
I wouldn't optimize this yet, because i don't see this happening often. It is needed for deleting entities for sure, when ORM sets the id to null. But why would users have a null value for a typed, non-null property themselves? That be a weird usage I don't recommend anyways
Fixes #34
Related to doctrine/orm#7999