5

I have found this question and the solution presented is something I figured out should work, but I'm staggered it seems to be the accepted practice.

It seems at odds with the idea of dependency injection; duplicating the list of dependencies for every class I'm interested in extending just seems wrong and leaves everything wide open to breaking.

I understand this is only a problem when I extend a class and want to take advantage of dependency injection in the subclass' constructor but that seems to be happening more and more as my Magento 2 projects develop.

Considering the above, and in the interest of asking a 'safe' question: Is there an alternative to __construct(MyCustomClass $myCustomClass)?

3

It seems at odds with the idea of dependency injection; duplicating the list of dependencies for every class I'm interested in extending just seems wrong and leaves everything wide open to breaking.

If extending classes seems wrong to you (for whatever reasons), the feeling is probably right.

Magento 2 has a plugin system that makes extending classes to change functionality almost never necessary. Use the interceptor plugins instead wherever possible.

Of course there are also classes that are meant to be extended, like the abstract block, model and controller classes. But those come with a Context object which holds all basic dependencies and makes it easier to override the constructor. See: What is Context Object purpose in any class's constructor DI ? How Context works?

General rules of thumb:

  • Observer > Plugin > Rewrite
  • Avoid extending classes to change behavior of existing objects
  • Must have overlooked this, seems to be a very powerful addition to Magento! In my underlying question I was hoping for something beyond extending classes and you delivered, so thanks for the input. – James Apr 12 '16 at 16:23
2

To me, I reckon that is the best practice and even if I agree with you that it doesn't look right and implies a high risk of error when calling the parent constructor.

I'm assuming that this is the best practice by just looking at the Magento 2 core code, taking one of the most (probably the most) used class of Magento: \Magento\Catalog\Model\Product. This class extends \Magento\Catalog\Model\AbstractModel.

And here is the constructor:

public function __construct(
    \Magento\Framework\Model\Context $context,
    \Magento\Framework\Registry $registry,
    \Magento\Framework\Api\ExtensionAttributesFactory $extensionFactory,
    AttributeValueFactory $customAttributeFactory,
    \Magento\Store\Model\StoreManagerInterface $storeManager,
    \Magento\Catalog\Api\ProductAttributeRepositoryInterface $metadataService,
    Product\Url $url,
    Product\Link $productLink,
    \Magento\Catalog\Model\Product\Configuration\Item\OptionFactory $itemOptionFactory,
    \Magento\CatalogInventory\Api\Data\StockItemInterfaceFactory $stockItemFactory,
    \Magento\Catalog\Model\Product\OptionFactory $catalogProductOptionFactory,
    \Magento\Catalog\Model\Product\Visibility $catalogProductVisibility,
    \Magento\Catalog\Model\Product\Attribute\Source\Status $catalogProductStatus,
    \Magento\Catalog\Model\Product\Media\Config $catalogProductMediaConfig,
    Product\Type $catalogProductType,
    \Magento\Framework\Module\Manager $moduleManager,
    \Magento\Catalog\Helper\Product $catalogProduct,
    \Magento\Catalog\Model\ResourceModel\Product $resource,
    \Magento\Catalog\Model\ResourceModel\Product\Collection $resourceCollection,
    \Magento\Framework\Data\CollectionFactory $collectionFactory,
    \Magento\Framework\Filesystem $filesystem,
    \Magento\Framework\Indexer\IndexerRegistry $indexerRegistry,
    \Magento\Catalog\Model\Indexer\Product\Flat\Processor $productFlatIndexerProcessor,
    \Magento\Catalog\Model\Indexer\Product\Price\Processor $productPriceIndexerProcessor,
    \Magento\Catalog\Model\Indexer\Product\Eav\Processor $productEavIndexerProcessor,
    CategoryRepositoryInterface $categoryRepository,
    Product\Image\CacheFactory $imageCacheFactory,
    \Magento\Catalog\Model\ProductLink\CollectionProvider $entityCollectionProvider,
    \Magento\Catalog\Model\Product\LinkTypeProvider $linkTypeProvider,
    \Magento\Catalog\Api\Data\ProductLinkInterfaceFactory $productLinkFactory,
    \Magento\Catalog\Api\Data\ProductLinkExtensionFactory $productLinkExtensionFactory,
    EntryConverterPool $mediaGalleryEntryConverterPool,
    \Magento\Framework\Api\DataObjectHelper $dataObjectHelper,
    \Magento\Framework\Api\ExtensionAttribute\JoinProcessorInterface $joinProcessor,
    array $data = []
) {
    $this->metadataService = $metadataService;
    $this->_itemOptionFactory = $itemOptionFactory;
    $this->_stockItemFactory = $stockItemFactory;
    $this->optionFactory = $catalogProductOptionFactory;
    $this->_catalogProductVisibility = $catalogProductVisibility;
    $this->_catalogProductStatus = $catalogProductStatus;
    $this->_catalogProductMediaConfig = $catalogProductMediaConfig;
    $this->_catalogProductType = $catalogProductType;
    $this->moduleManager = $moduleManager;
    $this->_catalogProduct = $catalogProduct;
    $this->_collectionFactory = $collectionFactory;
    $this->_urlModel = $url;
    $this->_linkInstance = $productLink;
    $this->_filesystem = $filesystem;
    $this->indexerRegistry = $indexerRegistry;
    $this->_productFlatIndexerProcessor = $productFlatIndexerProcessor;
    $this->_productPriceIndexerProcessor = $productPriceIndexerProcessor;
    $this->_productEavIndexerProcessor = $productEavIndexerProcessor;
    $this->categoryRepository = $categoryRepository;
    $this->imageCacheFactory = $imageCacheFactory;
    $this->entityCollectionProvider = $entityCollectionProvider;
    $this->linkTypeProvider = $linkTypeProvider;
    $this->productLinkFactory = $productLinkFactory;
    $this->productLinkExtensionFactory = $productLinkExtensionFactory;
    $this->mediaGalleryEntryConverterPool = $mediaGalleryEntryConverterPool;
    $this->dataObjectHelper = $dataObjectHelper;
    $this->joinProcessor = $joinProcessor;
    parent::__construct(
        $context,
        $registry,
        $extensionFactory,
        $customAttributeFactory,
        $storeManager,
        $resource,
        $resourceCollection,
        $data
    );
}

Yep, that's a huge constructor and the call to its parent constructor is also pretty big.

So, taking that class, which is to me, one of the most used class over Magento (apart from Abstract classes obviously and very high level classes), if the Magento team has not used a different solution at least to make that easier to read for us, I reckon that's because that's how it has to be done.

  • Very good point, with a good example which was actually spot-on relevant to my situation, thank you for the input. – James Apr 13 '16 at 8:18

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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