/dev/notes

There are only two hard things in Computer Science: cache invalidation and naming things.

-- Phil Karlton

I wanted to grab the last bit of a url that I knew would be the name of an image. I knew strstr well but that operates by giving you the remainder of a string that occurs after some needle in a string haystack. I wanted this behaviour, but only from the last instance of the needle.

strstr — Returns part of haystack string starting from and including the first occurrence of needle to the end of haystack.

strrchr -This function returns the portion of haystack which starts at the last occurrence of needle and goes until the end of haystack.

Let's look at how these work with an example

$url = 'http://www.google.com/a/b/c/d,img';
  echo strrchr($url, '/'); // prints /d.img  
  echo strstr($url, '/');   // prints //www.google.com/a/b/c/d.img
  

Now I've been programming in PHP for pushing on 12 years and this one still did my head in. The names of two very similar behaving functions bare little resemblance to each other.

At this point the arguments and criticisms over the core API have been exhausted and there's little that can/will be done. But I do wonder if it would be worth creating an object library to encapsulate primitive functions such as String, Integer, Array, Float etc., I'm not sure how possible auto-boxing is with PHP, and indeed if it's even a good idea. But definitely some object wrappers would help ease this API pain.

I'm seldom surprised by some of the horrors under the Magento hood, but today's little gem takes some beating.

On a setup I administer, there are over 200,000 address records. When you view an order in the backend, and click 'edit address', the server grinds away and eventually dies either because it hits the max_execution_time limit or runs out of RAM.

You might see an otherwise meaningless error like this:

Fatal error: Allowed memory size of 268435456 bytes exhausted (tried to allocate 79 bytes) in /home/somewhere/public_html/lib/Zend/Db/Statement/Pdo.php on line 290
  

The cause for this, is the strange manner in which Magento searches for addresses in app/code/core/Mage/Adminhtml/controllers/Sales/OrderController.

/**
   * Edit order address form
   */
  public function addressAction()
  {
      $addressId = $this->getRequest()->getParam('address_id');
      $address = Mage::getModel('sales/order_address')
          ->getCollection()
          ->getItemById($addressId);
      if ($address) {
          Mage::register('order_address', $address);
          $this->loadLayout();
          $this->renderLayout();
      } else {
          $this->_redirect('*/*/');
      }
  }
  

The problem is in the $address->getCollection()->getItemById() chain. Magento creates and fully loads a collection of Address objects (which when you have 200k of them, takes a while). The final call in the chain, getItemById() takes the collection, iterates over it assigning each address to an array keyed by its entityId. It then returns any value which matches $addressId.

Now, there's another, really simple way to do the same thing. It doesn't involve instantiating 200,000 address objects, or iterating over them, or even using associative arrays. It's very familiar.

$address = Mage::getModel('sales/order_address')->load($addressId);
  

This one line does the same thing far more efficiently. Now, the thing that worries me, is I can't see any reason why they aren't doing this already? The change in code works, nothing appears to break and the speed boost is (obviously) immense.

So why is it not done this way?