10 worst Magento practices

magento

Wrong Magento implementation can have critical consequences as dropping conversion rates. How to save your eCommerce before bad practices? It should be in our best interest to optimize code and therefore improve user experience. PHP code issues can be easier to find and fix than infrastructure ones.

I will introduce you the 10 worst Magento development practices followed by detailed examples. I picked the most common and the most important ones. I hope this article will help many to save their eCommerce before bad practices and to optimize their code.

1. Overriding core files

Magento uses nice event hooking system, following the Observer Pattern, allowing additional functionality to be plugged in and out without modifying the core code.

Very often developers rewrite a core file when there’s a possible way to accomplish the functionality with an event observer. Magento provides a lot of events that are dispatched at different times. All of these events can be observed, and the objects provided by the events can be modified.

There’s is no possible way to rewrite the same core file more than once without creating extends chain. By using event observers multiple modules can exist at the same time without conflicting — many modules can observe the same event.

2. Collections – query results limitation

In order to improve code performance and scalability remember to apply limitation on the collection’s query results — it’s much better to do $collection->getSize() instead of $collection->count() or count($collection) because that ways, all of the items will be loaded from the database and than iterated.

3. Memory – fetching large result set

Method fetchAll() used to fetch and iterate over larger result sets will lead to a very long execution time, moreover PHP will probably run out of memory. The better solution is to fetch row by row using the fetch() method. For example:

Wrong:

$rowSet = $this->_getReadAdapter()->fetchAll($select);
foreach ($rowSet as $row) {
    //process row
}

Good:

$query = $this->_getReadAdapter()->query($select);

while ($row = $query->fetch()) {
    //process row
}

4. Counting array

PHP Function count() is fast but when used in a loop this changes really severely. Calculating the size of an array on each iteration of a loop, if the array or collection contains a lot of items, will result in much longer execution time. Counting size should be done outside the loop not on each iteration.

Wrong:

for ($i = 0; $i < count($array); $i++){
    // do something
}

Good:

$size = count($array);
for ($i = 0; $i < $size; $i++){
    // do something
}

5. SQL queries inside a loop

Very common bad practice is to load Magento models and process it inside a loop. Running SQL query is very expensive operation, and doing it in a loop tends to make it even worse. Instead of doing that we could use data collections to load models and then process it. We also need to be careful when working with collection to do not exceed memory.

Wrong:

foreach ($this->getProductIds() as $productId){
    $product = Mage::getModel('catalog/product')->load($productId);
    $this->processProduct($product);
}

Good:

$collection = Mage:getResourceModel('catalog/product_collection')
    ->addFieldsToFilter('entity_id', array($this->getProductIds()))
    ->addAttributeToSelect(array('name'));

foreach ($collection as $product){
    $this->processProduct($product);
}

6. Models loading

We need to remember that each time of executing load() method, separate query’ll be executed on a database. Also, loading multiple models separately, slows down our code. Models should be loaded only once, with a single SQL query.

Wrong:

$attr = Mage::getModel(‘catalog/product’)->load($productId)->getAttr();
$sku = Mage::getModel(‘catalog/product’)->load($productId)->getSku();

Good:

$product = Mage::getModel(‘catalog/product’)->load($productId);
$attr = $product->getAttr();
$sku = $product->getSku();

We also don’t always need the whole model to be loaded — we can do it only when its necessary. In other cases, load only attributes which are needed.

$productId = Mage::getModel(‘catalog/product’)->getIdBySku($sku);  

7. Cron misconfiguration

Many developers use Daemons but the most common way to schedule tasks is through Cron jobs. Magento has a cron.php file that allows developers to add a crontab node to modules configuration. You must ensure that your system has a cronjob for doing it that way.

<crontab>
    <jobs>
        <catalogrule_apply_all>
            <schedule>
                <cron_expr>0 1 * * *</cron_expr>
            </schedule>
            <run>
                <model>catalogrule/observer::dailyCatalogUpdate</model>
            </run>
        </catalogrule_apply_all>
    </jobs>
</crontab>

8. Invalid cache usage

Speed up your Magento project by caching View parts. The HTML output of a block can be saved in the Magento cache. There’re nearly no blocks saved in the default Magento store, except the frontend and admin headers. The cache management has to be written in the constructor of the Block:

class {NS}_{Module}_Block_{View} extends Mage_Core_Block_Template {

    protected function _construct()
    {
        $this->addData(array(
            'cache_lifetime'    => 120,
            'cache_tags'        => array(Mage_Catalog_Model_Product::CACHE_TAG),
        ));
    }   

}

The output code will be saved in a cache until the “Product” cache will be deleted. If cache_lifetime is equal to false, it means that the cache will never expire.

9. Theme for mobile browsers

Good practice is to use optimized templates for mobile browsers. Magento can check Browser Agent and than serve a different theme, optimized for mobile. Magento allows to add exception to the most of the configuration parameters. It takes a regular expression string and compare it with the user’s browser agent.

You can use it to check what browser user’s using and set a different package, template, skin or layout. In other words, you can set a different them for mobile browsers.

Doing it is simple, just go to System -> Configuration -> General -> Design -> Package and Theme section and set-up appropriate settings.

10. Image Optimization

If you want to serve different images for mobile site, always declare width and height for them. The recommended image dimensions for mobile Magento store are:

  • Product image – 250px / 250px
  • Thumbnail – 120px / 120px

Don’t make the dimension larger and make sure that all your product images are of the same dimensions for height and width.

Photo by Robert Agthe

Looking to scale-out your
web application?

Hire Octivi!

Web developer

  • MistiDFox

    I just got paid
    ——————————————————-

    OPEN THIS LINK–>>­OPEN NEXT TAB FOR MORE INFO AND HELP

  • Dusan Lukic

    Thanks for the great article, I think that all bad practices are covered.

    However I am interested in finding out why do you think that getSize() is better than count()? I agree it’s better if you intend to call it before the collection is loaded, but getSize() from Varien_Data_Collection_Db makes another database query which is not necessary, and may be slower than actually counting items if your collection is already loaded.

    Also, getSize() basically takes your collection query and modifies it in order to get count. That will not work if you have GROUP BY in your query for example.

    So I just would not say it’s always better, depends on a case in my opinion.

    • Rafał Lorenz

      Thanks for your answer, you mention a good point, in your case you are right and most developers use $collection->count() or count($collection) it appear to be obvious solutions, developers want to retrieve the number of items, without processing. It’s wrong because all the items of the collection will be loaded from db and iterated. Its more efficient to call getSize() where you can specify limitation and offset $collection->getSelect()->limit(). And this was my point in this article 🙂

  • Ajith Shankar

    Thanks for your tips. Really useful for magento developers. A must read article.

  • great pointers!

  • Pedro Tentugal

    In your example of a loop, I think you either didn’t put in the correct prespective or something might be missing. If you execute the fetch for each row, I think it will be slower as you’re making multiple database access when you could fetch the whole data in chunks and iterate those chunks in a paginated page.