4

So, the new dependency injection approach requires me to inject everything I need into the contructor of an object. I get that.

But how is this more flexible?

For instance...

I have my own Topmenu class in my own module. It extends \Magento\Theme\Block\Html\Topmenu.

In my custom code I am missing the DataObjectFactory. So I created my own constructor. There are two issues that really bug me:

  1. I have to copy the entire parent constructor into my own just to inject one additional object.

This is for real? It is not that I am stupid and did not get the point? I really have to do it this way? To me this is just tedious and extremely redundant. But if I don't do it, PHP just stops with an error. How is this an advantage?

  1. Now that I have copied the constructor...

public function __construct( Template\Context $context, NodeFactory $nodeFactory, TreeFactory $treeFactory, array $data = [] ) { parent::__construct($context, $data); $this->nodeFactory = $nodeFactory; $this->treeFactory = $treeFactory; }

... I notice that $this->nodeFactory and $this->treeFactory are declared private. How am I supposed to extend a class/block this way?

And all this fuzz so I can have a simple dataobject that I can pass to an event I need to dispatch from my custom Topmenu.

Am I missing something here?

3 Answers 3

5

You're not stupid, you're just missing a few basic things here. But I think once you put them into practice you'll see that this is way more flexible.

Factory Classes Any class with Factory at the end, unless you explicity create a php file and class, will be automatically generated. For example, in your constructor, if you pass in \Vendorname\Modulename\Model\SomethingFactory, that class will get generated automatically.

In developer mode, the factory class gets created on script run/page load. In production mode, the factory class gets created during setup:di:compile.

A call to $this->somethingFactory->create() will return a new instance of \Vendorname\Modulename\Model\Something.

Now if you were not to pass a factory into your constructor but instead, just the class \Vendorname\Modulename\Model\Something, you would not be getting a new instance of that class but rather an instance that was already created. If you worked on M1 at all, you may remember Magento's singleton, its the same thing.


Regarding passing your class into the constructor, I see an optional $data = [] argument which is there for your convenience. So in your di.xml file, you could do something like:

<type name="Whatever\Class\You\Are\Passing\Into">
    <arguments>
        <argument name="data" xsi:type="array">
            <item name="my_class_factory" xsi:type="object">Vendorname\Modulename\Model\SomethingFactory</item>
        </argument>
    </arguments>
</type>

Then, in your class, depending on whether you've extended \Magento\Theme\Block\Html\Topmenu or you've created a plugin, you can either do

$object = $this->getData('my_class_factory')->create()

OR

$object = $subject->getData('my_class_factory')->create()

This is only the tip of the iceberg. I would strongly recommend the the following:

  • Follow Alan Storm's tutorials for Magento 2. I recommend all of them but specifically anything around dependency injection, argument replacement, and object management.
  • My personal favorite is the virtualType which allows for minimal code writing in favor of configuration.
  • Read up on Composition over Inheritance. Magento recommends against inheritance and abstraction which is why we see many interfaces throughout the codebase. The class extension and override of the constructor that you mentioned above goes against that principle. Obviously, there are times where it DOES make more sense to inherit but in most cases it's worth the extra effort in not extending a class. And if you do, try not to override a constructor bc as many of us learned when upgrading to 2.2.4, constructors change and then our modules break.

Good luck and let me know if you have any questions!

8
  • Thank you for your insights. Eye opening. I tried your approach using the $data array but it doesn't seem to work: <type name="OM\Foundation\Block\Html\Topmenu"> <arguments> <argument name="data" xsi:type="array"> <item name="data_object_factory" xsi:type="object">Magento\Framework\DataObjectFactory</item> </argument> </arguments> </type> But $object = $this->getData('data_object_factory')->create(); does not return anything. I did a bin/magento setup:di:compile and cleared all caches.
    – omsta
    Commented May 30, 2018 at 11:49
  • I'll give it a quick test myself but I do use this all the time. Are you doing your test in developer mode or production mode? Commented May 30, 2018 at 11:55
  • Also, where are you grabbing $this->getData('data_object_factory')? If you are doing it in your constructor before the parent::__constrcut($data) line then it makes sense that you haven't gotten it because that would be a call to the class's data array before it's had a chance to be added Commented May 30, 2018 at 12:00
  • Developer mode. I removed the constructor from my class completely now. $this->getData('data_object_factory') is being called in _addSubMenu.
    – omsta
    Commented May 30, 2018 at 12:13
  • Ok, checking into this now Commented May 30, 2018 at 12:17
1

You're not technically missing anything, what you're describing is exactly correct and you're also not the first to find fault in the way Magento is using the dependency injection pattern. It's being worked on. That said, there will always be inheritance injections, that's just the way it works.

As to WHY dependency injection like this? Because it makes for much easier and solid testing and decoupling of code.

The concept of DI is great and I would recommend delving into it. The internet provides plenty of info on design patterns, or check out Elements of Reusable Object-Oriented Software an amazing book on the topic.

0

Thanks to Shawn I found a way.

For some reason the di way simply does not want to work on my end. Since my original intention was to get a simple dataobject I could pass to an event I now do it the way it is done in Magento\Theme\Block\Html\Topmenu:

$transportObject = new \Magento\Framework\DataObject(['html' => $html]);
$this->_eventManager->dispatch(
    'page_block_html_topmenu_gethtml_after',
    ['menu' => $this->getMenu(), 'transportObject' => $transportObject]
);
$html = $transportObject->getHtml();

Contrary to magento's own recommendation they actually instatiated the object via new operator. I ended up doing the same.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.