Misuse of helper classes in Magento?
In its MVC structure, Magento allows you to throw in additional classes that might not fit in strictly under the MVC “letters”. One example of these classes are helper classes. These are usually used to isolate a certain, possibly useful, algorithm. Algorithm whose usage would be overhead to write a model as it is more static and general in nature, meaning there is no point in applying it to given instance of an object.
Calling a helper class from almost anywhere in your PHP code is extremely easy.
$helper = Mage::helper('catalog');
//which is same as
$helper = Mage::helper('catalog/data');Most helpers in Magento inherit from Mage_Core_Helper_Abstract class, which by itself has several “different responsibility” methods implemented.
If you remove the fancy “helper” name out of the context of thinking, helper is just a class like any other on OOP. So, how do helper classes conform to the 5 principles of class design within the paradigm of object oriented programming?
- (SRP) The SingleResponsibilityPrinciple
- (OCP) The OpenClosedPrinciple
- (LSP) The LiskovSubstitutionPrinciple
- (ISP) The InterfaceSegregationPrinciple
- (DIP) The DependencyInversionPrinciple
Witouth going to deep into this, I would like to point out single example/principle that caught my eye.
The SingleResponsibilityPrinciple among other things states: each responsibility should be a separate class, because each responsibility is an axis of change.
Let take a look at Magento’s core helper classes like app/code/core/Mage/Core/Helper/Data.php, where you have methods like formatPrice($price, $includeContainer = true), xmlToAssoc(SimpleXMLElement $xml), getStoreId($store=null), etc. Clearly this breaks the “each responsibility should be a separate class” logic.
This however does not play any significant role in your Magento development as one can look at helper classes simply as “place it here if you do not know where”. My reason of mentioning the above is to simply point out some paths one can consider when giving the “Magento is designed good/bad” statements.
Depending on the way you use them, helper classes can be truly useful. For example, code reuse in a company modules. Lets say a company has a certain policy on how its modules will behave in certain situations and what information they will share across various modules, etc. This is where you can throw in bunch of mini task helper functions/methods that do not seem to fall under any strict model definition, but preferably each helper class with its own subset of methods.
On the other hand, they can turn out to be a real mess of spaghetti code leading to bad application/module design. Seems like in Magento development it usually comes down to how much time you have on hand.
So, final questions: Are Magento helper’s good or bad thing? Where do you draw the line between block/view or model “level” helpers?
Looking forward to your replies.
Cheers.
8 comments
If a functionality is ‘stateless’, I will use helpers. If parameters alone are influencing the code path then it should be in helpers, otherwise it belongs some where else.
If code is mostly controlling display. For example if a helper is outputting html tags, it perhaps belongs in a template.
I have one doubt. Which way is better to use model function in magento Module/Block/file.php or Module/helper/data.
php?
In the end keep your business logic out of controllers, and put them in a model or helper is my keep it simple stupid rule of thumb, since the definition of a helper is pretty loosely based. According to Wikipedia: Helper classes are a term given to classes that are used to assist in providing some functionality, though that functionality isn’t the main goal of the application…
It appears obvious to me that Magento isn’t strictly speaking built upon the MVC model, because the abstractions here are taken much further. There are many confirmations for this, such as Blocks, Helpers and the fact that there’s a difference between the Models and the Resource Models. To me, the question brought up in this post is whether the Model should be used to represent an object, or a problem domain. I believe that the problem domain is represented by the a module itself. However, since there’s no object to represent a logical module, this function should be shifted onto the Model. This way, the Model represents both, thus providing an object-oriented way to represent a problem domain. From this perspective, it seems obvious to me that the Helper should represent a collection of methods, that could persist, but does not represent any problem domain as a whole.
@Jeffry. My Model/Helper guideline isn’t about persistance, it’s more about complexity. Once I start writing some code that’s involved enough that I want to store a value in a property, even if only for the length of the request lifecycle, it’s a sign to me that I’m modeling **some** problem domain, and it’s probably time to consider it as such.
It’s ok to use helpers together with MVC because MVC is simply a pattern and it’s ok to use patterns together with other patterns. Heck, you could use patterns within patterns if you want. Singleton+Factory anyone?
The observation that a helper becomes a model once it’s needs persistence is wrong. Your models represent your domain and thus your business logic. You can perfectly have objects within your domain model which never get stored. Helpers usually, not always, are useful to remove duplication within your views. If you remove duplication from a model with Extract Method and stuff into a helper; you might want to try Extract Superclass.
I don’t know if it’s useful to think in terms of SOLID principles when it comes to magento. There is very little which can be fixed. 450k SLOC with very few tests no mere mortal can refactor, not even chuck norris
In working with magento (and any other framework) though; I think the only principle you still have control about and really should pay attention about is the open closed principle. You guys write about it all the time. Never, ever thouch core code (closed for modication) but inherit instead ( open for extension )
Another rule of thumb: If I need exactly same function in multiple places that do not inherit from single base class, I put them in helpers. Quite often that’s because I might need something really simple in two different models and/or both model and controller (and/or block, template, exception…).
We also have module that’s only a collection of helpers that we use in many different modules. Most of these are simple functions for validation, formatting, parsing, conversion and so on.
Both of these are made to ease development and make it faster, not because it would (or wouldn’t) be mvc way of doing things. Many of the things in the helpers could be models if we would stick strictly to mvc but that wouldn’t make development easier or faster for us.
Simple rule of thumb: Whenever I’m tempted to add a methed to my controller that isn’t an action, I create a helper instead. When that helper starts needing to store properties to do its job, its turned into a Model.
However, that’s not the right answer, and there isn’t one. It comes down to the patterns you and your team work most effectively with and allow you to quickly achieve your goals. Look deep enough into the Magento source and you can see that even the Core team has evolved its approach over the years.
Don’t get too caught up in The One True Way™, you’ll be a better team for it.