Closed oprypkhantc closed 10 months ago
Yes, it makes sense. I think these magic methods were added initially as a way to keep "clean" objects. But with recent improvements to PHP's class syntax, they're really unnecessary. Nonetheless, I agree with your take that if the method exists it should take precedence.
Hey.
Currently
PropertyAccessor
checks for the existence of magic__call
method, and if it exists, attempts to callgetFieldName()
orsetFieldName()
methods, even though the field was defined as a property. What's worse is that this checks seems to be broken for__call
:Such code results in a
ReflectionException: Method SomeDto:getSomething() does not exist in /app/vendor/thecodingmachine/graphqlite/src/Utils/PropertyAccessor.php:111
because it attempts to check whether methodgetSomething
is public without checking if it even exists.This is trivial to fix, but while we're here, why does it even try to call
__call
? In my case__call
only exists for backwards compatibility with older style properties that had getters, so__call
just directs all$this->getSomething()
to$this->something
, but I'd imagine it doesn't work that way in all cases, so it seems strange to me that__call
has the priority over setting the property directly.I'd advocate that
graphqlite
should follow the general logic of PHP here: if a "hardcode" property/method exists, it's always used and magic methods are never called. In this case, I'd expect it to first check for a real getter method, then a real property, and only then a magic setter method.Does this make sense?