Should Magento controllers be declared FINAL?

In true Magento confusion there are two sets of controller classes; Those in the Controller directory and those in the controllers directory. The former have names like Mage_Adminhtml_Controller_Action and the latter are Mage_Adminhtml_IndexController. The latter are normally descendants of the former.

Because the latter don’t cleanly map their names to their location in PEAR naming convention they don’t get autoloaded by the normal autoloader, you cannot include them from your scripts since you cannot be certain which code pool they are in and it potentially breaks the compilation feature.

I feel because it’s so hard to extend them they should be officially final and be done with it. Will this break anything I’m not thinking of? Is it bad practice somehow? Can anyone suggest better terms for describing the extendible classes and damn-hard-to-extend classes?

2 thoughts on “Should Magento controllers be declared FINAL?”

  1. That would be awful.

    If you went back in time maybe you could make a case for declaring those controllers final to avoid the problems you’re talking about, but introducing a change like that would significantly break a large number of Magento installations out in the wild. There’s numerous cases where extending a base controllers for new store functionality (or even override/rewrite functionality) is the right move. Making them final now means you not only break applications, you break them in a way that’s impossible to recover from.

    Even if you went back in time, I’m not sure your frustrations (although understandable) hold up as reasons

    No Autoloading

    This sounds like more of a case for fixing and/or adding an additional autoloader

    Cannot include them from your scripts since you cannot be certain which code pool they are in

    I’m not sure I follow the concern here. Including a base controller is as simple as

    require_once('Mage/Catalog/controllers/CategoryController.php');
    

    No, you’re not certain which code pool they’re in … and that’s fine. The point of code pools is they have fallback PHP include paths, allows users to use their own version of a particular class if need be without wrecking the core.

    Potentially breaks the compilation feature

    I’m not familiar with the problem being described here, but again, it sounds like more of a case to extend/fix the compiler’s functionality

    Making controller classes as final to fix these problem would be like using a bazooka to handle a problem with house mice. The whole reason we have controller classes is so other URLs on our application can inherit functionality. Even if you don’t believe in that for projects where you’re the architect, taking that away after granting it is, at best, a bad move.

  2. people should use magento methods for getting the path for controller class and then it would’t break the compiler as the methods would return the correct path.

    require_once  Mage::getModuleDir('controllers', 'Mage_Checkout').DS.'OnepageController.php';
    

Leave a Reply

Your email address will not be published. Required fields are marked *