Magic getters on Varien_Object
(M1) and DataObject
(M2) are common practice, but with Magento 2 it feels wrong to use it.
Good:
- easy to read/write
Bad
- It causes problems when using digits in keys (see: Magento 2: different way get field of a collection or Get Custom Product attribute using camel case)
- code analyzation tools complain about non-existing methods
Question
With Magento 2 we have two new methods:
getDataByKey($key)
getDataByPath($path)
Is there any good reason to still use getData($key)
or any magic getters?
Edit:
@Vinai thanks. I did not mention the @method
method, because my approach was quite different.
It only helps the IDE, but has no impact on other things.
There are several mergedf PRs on that are "micro-optimizations" like casting to (int)
instead of intval()
or get array size outside loops (even for small arrays).
At the other hand there are
-
magical getters, that have some "overhead" as Marius described ….
strtolower(trim(preg_replace('/([A-Z]|[0-9]+)/', "_$1", $name), '_'));
-
getData($key)
mehtods also have to 2-3 additional checks …if ('' === $key) {
if (strpos($key, '/')) {
if ($index !== null) {
For own code a totally agree to prefer real methods, but in same cases it's not possibly … e.g. you've created a custom event …
$value = $observer->getVar_1();
$value = $observer->getData('var_1');
$value = $observer->getDataByKey('var_1');
Using 3rd with /** @var some $value */
seem best to me. (?)
Best Answer
The above question is about using magic methods vs.
getDataByKey
orgetDataByPath
. I think there is a third option, too, and that is implementing real getter and setter methods.The
getData*
methods all have the downside that they have to be annotated for type inference to work.Usually that is done with a
/* @var string $foo */
annotation above thegetData*
call.This is a bit smelly, because the type of the data should be declared in the class that contains the data, not the class that calls
getData*
.The reason for that is that if the data changes, the class is the most likely to be updated, not all
getData*
call sites.That is why I think real methods increase maintainability compared to using
getData*
accessors.So I think it boils down to a trade off between maintainability and quicker implementation (less code to write).
Luckily, nowadays IDEs are really good at creating the getter and setter implementations for us, so that argument doesn't really apply any more.
One more argument against magic getters and setters that is missing from the question above is that it's not possible to create plugins for them.
The only other value I think I can add to the topic is trying to collect the reasons to use or not to use
@method
annotations, if implementing real methods is out of the question for some reason.Pros
@method
annotation is a little less code to write compared to implementing a real getter and setter. This is barely true though nowadays because IDEs are good at generating accessor methods, so that's not a real benefit any more.Cons
@method
annotation and a real method with the same name exist, the annotation type signature overrides the real method during static code analysis, which is the opposite of what the PHP interpreter does. This again can easily lead to subtle bugs.For the above reasons I personally do not use
@method
annotations if I can avoid them.For code that is intended to live long I implement real getter and setter methods. The maintainability gain is worth the effort of triggering the IDE to generate them.
For more experimental code during a spike, or for a simple implementation detail of a module I use
getData*
methods, too, because I'm lazy.