Question of previous developers use of <rewrite>'s

Question

I'm not only new to Magento, but also inheriting a project from a previous developer. The developer has a module called Adminhtml, which he used to customize the Adminhtml section, and inside the config.xml file we has defined the following re-writes:

    <blocks>
        <adminhtml>
            <rewrite>
                <catalog_product_edit_tabs>WG_Adminhtml_Block_Catalog_Product_Edit_Tabs</catalog_product_edit_tabs>
                <dashboard>Catalog_Adminhtml_Block_Dashboard</dashboard>
                <catalog_product_edit_tab_attributes>WG_Adminhtml_Block_Catalog_Product_Edit_Tab_Attributes</catalog_product_edit_tab_attributes>
                <catalog_product_edit_tab_settings>WG_Adminhtml_Block_Catalog_Product_Edit_Tab_Settings</catalog_product_edit_tab_settings>
                <catalog_product_edit_tab_attributes_create>WG_Adminhtml_Block_Catalog_Product_Edit_Tab_Attributes_Create</catalog_product_edit_tab_attributes_create>
                <catalog_helper_form_wysiwyg>WG_Adminhtml_Block_Catalog_Helper_Form_Wysiwyg</catalog_helper_form_wysiwyg>
                <catalog_product>WG_Adminhtml_Block_Catalog_Product</catalog_product>
                <catalog_product_grid>WG_Adminhtml_Block_Catalog_Product_Grid</catalog_product_grid>
                <catalog_product_edit>WG_Adminhtml_Block_Catalog_Product_Edit</catalog_product_edit>
            </rewrite>
        </adminhtml>
    </blocks>

My question is is this the best way to do this? My suggestion to my supervisor was breaking this into a dedicated module we would call "Showcase", which is basically assembling a customized grid. Is the second way I proposed the acceptable way?

Answer 1

Without knowing what these rewrites are doing, it's hard to say but I have a feeling that this was not the best way to accomplish whatever he wanted to.

Red flags:

• The module name "Adminhtml" doesn't give anything away, it looks like everything related to the backend has been thrown together into one module. Better: multiple small modules that have one single responsibility and do not depend on each other
• Many rewrites. Class rewrites are a last resort and you should always try to find a solution without them, preferable using observers or layout updates. Unfortunately there are many cases where there is just no other way to change behavior in Magento but this many rewrites in one module are a sign that somebody was too easy going with them or doesn't know how to leverage the better alternatives. Especially block rewrites can often be avoided with layout updates Better: as said, use more robust alternatives wherever possible. Class rewrites are always potential conflicts (with other modules or with Magento updates)
• Event the rewrites are not done right, judging by your comment "The code he copied over is just a full copy of what is in core/Mage and seems a little silly to copy the entire file" - yes, and besides silly, it's not update-safe, it's not readable and it doesn't encourange minimal changes. Better: A rewrite should extend the original class and only override single methods. If possible, those should also not be copies of the original but call the parent method and add code before or after that.

Answer 2

Without looking at all the code, it is hard to say yes or no. On the one snippet you provided, the developer at least knew how to create redirects.

You do have a good point of breaking it into a separate module, that way you wouldn't need all these rewrites. However I would step into his shoes and figured out why he did it this way. Option 1 and option 2 are both correct to do, however you need to do a pro/con chart for each decision. This is more of a developer/business decision.

Since you are new to Magento, I would recommend you keep it as is. Keep it simple while you get use to everything. You don't want to try and over engineer something when you are first starting out.