Skip to content

Creating a Custom Helper Class with Coding Best Practices ? Code Review Request

Adobe Commerce ver. 2.4.6-p2, multi-site instance

Inheriting code base with numerous custom modules that are not scoped – but need to scope them as individual sites are migrated to new frontend stack

Created a custom module to call a StoreManager method, getCode(), to return the website code and then use php if statements to match the website code, then execute or skip executing parts of templates, etc.

I have created a new class (Data.php) within a new module at: appcodeMyCoolVendorNameMyCoolNewModuleHelper

<?php
    
declare(strict_types=1);
    
namespace MyCoolVendorNameMyCoolNewModuleHelper;
     
class Data extends Template {
            
   public function getWebsiteCode()
   {
      return $this->_storeManager->getStore()->getCode();
   }
}

Then in any template I need to scope away from a particular site, I add the following:

<?php

declare(strict_types=1);

use MyCoolVendorNameMyCoolNewModuleHelperData;

/** @var Data $block */

$websiteCode = $block->getWebsiteCode();
?>

<?php if ($websiteCode !== 'my_cool_website_code'): ?>

// Some previous code that we do not want to execute on specified site, 
// but if removed completely may break functionality on other sites

<?php endif; ?>

And this is working fine.

I have seen this implementation suggested several times, however, the Template.php says to avoid extending Template class. So if composition is preferred over inheritance, should this be refactored as:

<?php

namespace MyCoolVendorNameMyCoolNewModuleHelper;
 
class Data
{
    private Template $template;
    
    function __construct(Template $template)
    {
        $this->template = $template;
    }

    function getWebsiteCode(): string
    {
        return $this->template->_storeManager->getStore()->getCode();
    }
}

And this is also working fine. And dependency injection is being followed.
However, this is verbose and repetitive and does not exploit syntactical sugar of PHP 8’s constructor property promotion. So could be refactored again:

<?php

namespace MyCoolVendorNameMyCoolNewModuleHelper;
 
class Data
{
    function __construct(
        private Template $template
    ) {}
    
    function getWebsiteCode(): string
    {
        return $this->template->_storeManager->getStore()->getCode();
    }
}

And this is the same as above but more concise. However, instead of depending on the Template class, it should depend on an interface implementation of Template. So instead should implement a TemplateInterface and specify the preference for this new interface in di.xml. So should be rewritten as:

<?php

declare(strict_types=1);

namespace MyCoolVendorNameMyCoolNewModuleHelper;

use MyCoolVendorNameMyCoolNewModuleApiTemplateInterface;
 
class Data implements TemplateInterface
{
    protected $websiteCode;

    public function getWebsiteCode()
    {
        return $this->template->_storeManager->getStore()->getCode();
    }
}

..and specify the preference for this new TemplateInterface in a di.xml:

<config xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:noNamespaceSchemaLocation="urn:magento:framework:ObjectManager/etc/config.xsd">
    <preference for="MyCoolVendorNameMyCoolNewModuleApiTemplateInterface" type="MyCoolVendorNameMyCoolNewModuleHelper" />
</config>

Is this last interface implementation shown above correct and does this follow best coding practice? I ask because I have not seen an example written this way—only first way shown above with inheritance (class A extends B)