2

Is it safe for a model method to rely on the model being in a loaded state i.e. accessing a data value?

An simplified example:

public function testMethod()
{
   return 100 x $this->getData('test_attribute');
}

This method only makes sense if the model had loaded correctly.

Is this bad practice? Or any alternative?

8

The standard way to test if a model was loaded is

$model->load(...);
if ($model->getId()) {
    // ... model loaded ...
}

Or, if you are in the context of the model class, use if ($this->getId()) accordingly.

  • testMethod in my example is inside the model itself. But it relies on data that will only be on the model after it is loaded correctly – Marty Wallace Jun 12 '13 at 15:02
  • In that case it is customary to use if ($this->getId()) to check if the model was loaded correctly. Of course you can use any other attribute that you can rely on to have a value that evaluates to true in a boolean context, too. – Vinai Jun 12 '13 at 15:04
  • Yea the syntax im fine with, its the concept behind it i was not sure about i.e. whether a model should have to check if it is loaded / has an id or if there is a more elegant / standard magento way to do this. Now you have confirmed this is the way – Marty Wallace Jun 12 '13 at 15:10
2

No, it's not safe. If a dependency is required, it should validate that dependency:

public function testMethod($product)
{
   if(!$product instanceof Mage_Catalog_Model_Product){
      Mage::throwException('/*meaningful exception*/');
   }
   return 100 x $product->getData('test_attribute');
}

Another way is type-hinting:

public function testMethod(Mage_Catalog_Model_Product $product)

Alternatively, if you're relying on properties set you should use a protected getter:

public function getTestAttribute(){
    if(!isset($this->_test_attribute)){
        // then load it
    }
    return $this->getData('test_attribute');
}
0

There is standard _afterLoad() function you can use:

protected function _afterLoad()
{
    //you can set some model flag or fire some method here
    return parent::_afterLoad();
}
  • I prefer answer of @Vinai, but if you want to be sure 100%, try to use above – mageUz Jun 12 '13 at 15:41

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.