Posted on by Fabian Schmengler

Stop using Helpers

“Helpers” are often used as convenient collection of functions. They are also a sign of bad design, and I want you to stop writing them. I’ll quote myself

In general, having classes named “Helper”, “Util” or similar just says “I have some functions that I don’t know where to put” and don’t make much sense as a class.

It’s not very object oriented. Not at all to be frank. The idea of object oriented programming is that there are objects that send each others messages. They have an active role in the system and are not just containers for data and code, which would be a very procedural way to see them.

So what would be the role of a helper? A butler maybe, that does not act on its own and will do anything you tell him. But to do that, he needs access to your whole household, your bank account and your car.

In a system like Magento 2 with consequent usage of dependency injection this is even more evident. The more functions you add to a helper, the more dependencies it collects.

Example

I’ll give an example how typical helper functionality can live better in its own class. I recently added a feature to our Magento module IntegerNet_Anonymizer to exclude customers from certain email domains from being anonymized. The domains would come from a configuration value. I needed to check email addresses against these configured value at several places. Now the naive way would be to create a helper method like this in the module helper (in Magento 1.x each module has one default helper class)

    public function isEmailExcluded($email)
    {
        $excludedDomains = array_map(
            'trim',
            explode(',', Mage::getStoreConfig(self::CONFIG_EXCLUDED_EMAIL_DOMAINS)) ?: []
        );
        $emailDomain = preg_replace('{.*@(.*)}', '$1', $email);
        foreach ($excludedDomains as $domain) {
            if ($emailDomain === $domain) {
                return true;
            }
        }
        return false;
    }

What did I do instead? Put this logic in its own class ExcludedEmailDomains, with one method 

public function matches($email)

but without the dependency to the Magento configuration. $excludedDomains are passed via constructor instead.

Now this is not a “helper” anymore, this is an actual object with an identity. It represents a list of domains. Within the Magento module this would be a “model”.

Why is this good? Suddenly, it was really easy to unit test the logic (see ExcludedEmailDomainsTest): no dependencies, nothing to be mocked, no Magento involved.

And where I use it, I don’t need to buy a whole mixed lot of unrelated functions. Keep in mind that having many classes is not bad, having classes that do too much on the other hand, is a code smell, a sign of bad design.

If you look at the rest of the module, you’ll notice that I still added a method to the module helper, which acts as factory method. In the post quoted above, in the context of Magento 1.x, I also said:

However, I would not be too strict about “not using helpers at all” and instead use them for query shortcuts like:

  • access module configuration:
  • factory methods for library classes

You could find other places but IMHO, the module helper is often good enough for things like that.

So I actually won’t blame you for writing helpers, but please think before you do and keep them as stupid as possible. If your helpers contain any domain logic, you are doing it wrong.

Related Posts:

7 Replies to “Stop using Helpers”

  1. I’d have to disagree with you on this one. Having simple functions which are easily testable is much less of a code smell then using OOP/classes. Classes can get bloated very easily, and when pulling in a class, you typically pull everything else along with it. Not to mention it’s very easy to pollute your codebase with inheritance which will make your code much harder to test in the long run. There is nothing wrong with utility functions.

    1. I agree with you that inheritance can pollute your code, but inheritance is not inherent in OOP. I advocate a clean object oriented design with as little inheritance as possible and small classes. If you refactor utility functions and end up with bloated classes with many dependencies (“pull everything else along with it”), you are going the wrong way, too. It takes thought and discipline.

      That being said, you don’t have to go all in on OOP, especially in PHP. If you have pure functions with input and output, no hidden dependencies, and no side effects, you can implement them as actual functions. Okay, in Magento 1, I would probably still use a helper because functions cannot be autoloaded. But anything that uses Mage in M1 or injected objects in M2 is not a pure function and should not be in a helper.

      1. I agree with your last statement! 🙂 Once a function is no longer pure, it loses a lot of it’s effectiveness. You are better creating your helpers with using specific parameters instead of Mage/globals.

  2. A function! That is a more elegant solution 🙂
    A binary function in this case 🙂 no class, no new, no public, no private(what for?), no temporary instance, no function keyword(that creates a method instead, WEIRD!!!!), no messages, no this (make you a favor, and stop using $tmp, $this, $that, $a…).
    Just input, function, output.
    yournamespace\isEmailExcluded($domains, $email), WOW, that is simple. Don’t you want to repeat the $domains? Applicate it partially 🙂 The same if you need to compose the function and reduce the arity.
    $isIncludedInMyDomains = function($email) use($domains) { return isEmailExcluded($domains, $email); }
    And this ensures your input always produce the same output.

  3. Thanks for sharing. This was an insightful read, and it challenges my thinking about how I’ve developed previous modules for Magento 1.

    I recently developed a helper for M2, which was designed to be a companion to an attribute. Its goal is to read files from the `media` directory and translate them to human-readable labels, which are fed into an upgrade script to populate multi-select attribute options. As you might expect, I’ve configured a dependency, `Magento\Framework\App\Filesystem\DirectoryList` for the constructor of this helper in order to get the appropriate `media` path. While I could reasonably generate this path without framework code, it seems to me best practice to create the dependency on framework code here.

    Given the above case, are you suggesting that leveraging DI is a bad idea? If so, how would you have solved it differently?

    For additional context, note that I did not include this helper logic directly in my upgrade script, because it also has to be used on the frontend for reverse-translating attribute option values into filenames.

    1. Thanks for the question! The dependency to DirectoryList makes sense and as DirectoryList itself does not have further dependencies, it can be used in tests without problems as well:

      new DirectoryList('virtual/root/dir')

      It would be consequent to only take $pathToMedia as constructor argument, but then you are losing the power of automatic dependency injection in Magento 2, so there’s a compromise to be made.

      But let’s see if you can get turn the “helper” into something more meaningful. In Magento terms, isn’t this a source model? If not, I would consider something like this (assuming the files in the media directory are brand images):

      namespace Your\Module\Model;

class BrandImages
{
    private $pathToMedia;
    private $images = [];
    public function __construct(DirectoryList $directoryList)
    {
        $this->pathToMedia = $directoryList->getPath(DirectoryList::MEDIA);
    }
    private function load()
    {
        if (empty($this->images)) {
            foreach ( [ scan files in media directory ] as $filePath) {
                $image = BrandImage::fromFilePath($filePath);
                $this->images[$image->optionValue()] = $image;
            }
        }
    }
    public function all()
    {
        $this->load();
        return $this->images;
    }
    public function imageByOptionValue($optionValue)
    {
        $this->load();
        return $this->images[$optionValue];
    }
}

class BrandImage
{
    private $optionValue, $optionLabel, $filePath;
    public static function fromFilePath($filePath)
    {
        // convert file name into option value and human readable label here
        return new self($optionValue, $optionLabel, $filePath):
    }
    public function __construct($optionValue, $optionLabel, $filePath)
    {
        // assign attributes
    }
    // getters:
    public function optionLabel() { }
    public function optionValue() { }
    public function filePath() { }
}

      Using the “Model” namespace comes from the old convention where everything was either a model, a helper, a block or a controller, but in Magento 2 this is no technical restriction anymore and the core modules also use different namespaces now. So you could leave out “Model” as well or place the classes somewhere else, that’s a matter of taste.

Comments are closed.