Question

At this time I am annoyed of writing similar constructors en masse like the following within my modules.

public function __construct(
    \Magento\Framework\Model\Context $context,
    \Magento\Framework\Registry $registry,

    /* ... */

    \Foo\Bar\Model\Baz $baz,

    /* ... */

    \Magento\Framework\Model\ResourceModel\AbstractResource $resource = null,
    \Magento\Framework\Data\Collection\AbstractDb $resourceCollection = null,
    array $data = []
) {
    $this->registry = $registry;

    /* ... */

    $this->baz = $baz;

    /* ... */

    /* some awesome stuff */
}

In many many many cases I need instances of the same classes all over my module.

So I was questioning myself, if it would be an acceptable way to use one or two central helper classes, which provide the necessary classes, instead of defining them in every single constructor.

This means a pattern like this:

Helper Class

namespace Foo\Bar\Helper

class Main
{
    protected $baz;



    public function __construct(
        \Magento\Framework\Model\Context $context,
        \Magento\Framework\Registry $registry,

        /* ... */

        \Foo\Bar\Model\Baz $baz,

        /* ... */
    ) {
        $this->registry = $registry;

        /* ... */

        $this->baz = $baz;

        /* ... */

        /* some awesome stuff */
    }



    public function getBazInstance()
    {
        return $this->baz;
    }
}

The shorter constructor

public function __construct(

    \Foo\Bar\Helper\Main $mainHelper,

    \Magento\Framework\Model\ResourceModel\AbstractResource $resource = null,
    \Magento\Framework\Data\Collection\AbstractDb $resourceCollection = null,
    array $data = []
) {
    $this->mainHelper = $mainHelper;

    /* some awesome stuff */
}

At this point I am not sure if I will have to deal with big disadvantages in future caused by this structure. Would this be an acceptable way to reduce the amount of DI definitions?

Was it helpful?

Solution

Check out \Magento\Framework\Model\Context, referenced in your example. What you describe is exactly what it does. Magento uses similar Context objects throughout the core to shorten DI lists.

The only thing to keep in mind is that this should not be used to hide away bad architectural decisions. You should consider whether each of the classes you need 'all over your module' is really necessary, and if so, whether there's an alternate way of organizing your code that would accomplish the same goal better. It's easy to introduce unintentional performance issues.

OTHER TIPS

I'm pretty sure you're not the only one in this case and in some way I totally understand why you thought about doing this.

To me, the main issue I see with such approach is that you lose one of the main benefits of Dependency Injection which is to know straight away what your class depends on when checking the constructor.

Another important benefit of Dependency Injection is that it makes code easier to test in an automated framework. In your case that's definitely one downside.

Those are the two reasons that come up but there may be more.

EDIT: I'm just gonna to add a quote from Alan Kent (who is part of Magento) that you can find in the comments of this question:

I generally discourage throwing methods into a helper class which are not related. It is better to have separate classes that represent real purposes. Or use static methods in which case there is no need for a constructor (the calling code is responsible to get handles to needed data structures).

Licensed under: CC-BY-SA with attribution
Not affiliated with magento.stackexchange
scroll top