Magento Varien_Data_Collection returns wrong size after removing items

Question

I'm running on Magento 1.4, but also have verified the problem in 1.7.

Working with an instance of Varien_Data_Collection provides the use of Varien_Data_Collection::removeItemByKey. In my case, I'm removing items from the collection, and later trying to get the updated size of that collection, like so:

$body=$this->getTable()->getBody();

echo $body->getHeight(); // Outputs 25

$body->deleteRow(1);

echo $body->getHeight(); // Still outputs 25

...

My_Model_Body_Class extends Mage_Core_Model_Abstract {

  /* @var $_rows Varien_Data_Collection */
  protected $_rows;

  public function deleteRow($index) {
    $this->_rows->removeItemByKey($index);

    return $this;
  }

  public function getHeight() {
    return $this->_rows->getSize();
  }

}

...

Code limited for brevity.

So if you call my deleteRow method, the item will in fact be removed from the collection, but subsequent calls to get the size of that collection will always return the original count. Therefore, if I have 25 items in the collection, and remove 1, then a call to getSize on the collection returns 25.

I traced this back to the parent class, in Varien_Data_Collection::getSize:

/**
 * Retrieve collection all items count
 *
 * @return int
 */
public function getSize()
{
    $this->load();
    if (is_null($this->_totalRecords)) {
        $this->_totalRecords = count($this->getItems());
    }
    return intval($this->_totalRecords);
}

We see that the count hinges on the NULL status of the _totalRecords property. So it looks like a bug in core code. Am I reading this correctly? Should I just rely on a call to count on the items?

Answer 1

We see that the count hinges on the NULL status of the _totalRecords property. So it looks like a bug in core code. Am I reading this correctly?

Whether to interpret said behaviour as bug or feature, lies in the eyes of the beholder.

This behaviour is not 1.4 specific, btw; it works the same way up to the current CE version (1.8.1).

public function getSize()
{
    $this->load();
    if (is_null($this->_totalRecords)) {
        $this->_totalRecords = count($this->getItems());
    }
    return intval($this->_totalRecords);
}

Most people for sure expect a method named getSize() to always return the current size, so they may call it a bug, perhaps.

But if you take a closer look at the Varien_Data_Collection class, you'll notice, that getSize() is not the only method that looks somewhat.. "weird".

Take the addItem() and removeItemByKey() methods, for example.

Why don't they increment/decrement the _totalRecords property, when getSize() uses it?

Lazy Loading

The reason for these "weird" behaviours is, that Varien_Data_Collection basically is designed for the usage of the Lazy Loading pattern. That means, it allows to delay loading of the collection, until the data is really needed.

To accomplish this, Varien_Data_Collection implements the IteratorAggregate and Countable interfaces. Their implementation points are the getIterator() and count() methods:

public function getIterator()
{
    $this->load();
    return new ArrayIterator($this->_items);
}

public function count()
{
    $this->load();
    return count($this->_items);
}

As you can see, both of these methods call load() first.

The result of this is, that whenever you use foreach or the PHP function count() on the collection, the load() method automatically will be called first.

Now, for a default Varien_Data_Collection nothing special will happen, when load() is called, because load() only calls loadData() and loadData() only returns $this.

But when it comes to its heavily used child class Varien_Data_Collection_Db, then the call to load() will result in loading the collection and setting _totalRecords to the SQL count of loaded records.

For performance reasons the collection usually will be loaded once only (if it hasn't been loaded yet).

So you see, depending on which context you use Varien_Data_Collection in, it perfectly makes sense, why getSize() behaves this way.

Should I just rely on a call to count on the items?

If your collection is not bound to complex or slow I/O, I'd say: yes.

You can simply use:

$n = count($this->_rows->getItems());

Or even this way:

$n = count($this->_rows->getIterator());