Stop using Helpers

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.

4. Helpers are abstracts, simple as that.

Comments are closed.