[WIP] Add Interfaces to models

[sreichel]

Description (*)

Thanks @kiatng, for frequently reviewing phpstan/DOC-block PRs. Several times you referred to the table structure - to match column types.

How about to add interfaces for models?

• we work with the correct tpye
• it replaces some call to magic methods (reason for the PR) (1)

Note (1):

Magic methods are nice, but it slows down OpenMage. Not with a single call, but it sums up.

Reasons

You can call get or set on every Varien_Object, and $this->getSomething()) looks good.

Downside - it requires __call and some more checks. For every magic method call 3-4 other methods are called.

How to make it "better"?

Minor improvement. Instead of using $this->getData('some_string'), better use $this->getDataByKey('some_string'). This saves 2 small checks - every time. (It sums up.)

Having @method annotations is nice for IDEs, or when working with "strict_type=1", but the additional 3-4 calls are left.

To avoid calling __call, we have to use real methods instead. Nothing else.

For tests i added interface and corrospending getter/setter for ...

• Mage_Cms_Model_Block
• Mage_Cms_Model_Page
• Mage_Core_Model_Website
• Mage_Log_Model_Visitor

... and did some profiling.

Sample data. Index page. Only for adding real methods (and avoid duplicated method calls) it saves 230 function calls every time.

(ignore WT, CPU, Mem)

Note: the is a possibly change to break 3rd-party code by of type-hinting in interface-methods [,but that should be okay].

Related Pull Requests

• see Updated Varien_Object::getData() and added getDataByKey() & getDataByPath() #4205

[kiatng]

Instead of interface, how about new abstract classes for the real methods, sandwich between the model class and Mage_Core_Model_Abstract`. BC?

[sreichel]

The abtract class would only be there to remove the methods related to entity to another class. (Not?) I dont see the benefit.

The interface is independent of it. It only defines the methods to get/set data from DB.

[kiatng]

Based on my limited experience on OpenMage. Interface is great for adapters and makes it easier to implement new adapter, it just needs to fulfill the interface for it to be like plug-n-play, swap-in-n-out. For abstract classes, I am thinking that it can unclutter the real methods from other logic. For example, I can have different set of attributes for each customer group, I can have subset of groups:

abstract class Company_Module_Model_Applicant_Abstract extends Mage_Customer_Model_Customer { 
// real methods on Applicant attributes 
}
class Mage_Customer_Model_Applicant extends Company_Module_Model_Applicant_Abstract { 
// logic related to Applicant 
}
abstract class Company_Module_Model_Applicant_Student_Abstract extends Mage_Customer_Model_Applicant { 
//  additional attributes + inherit Applicant logic and attributes 
}
class Company_Module_Model_Applicant_Student extends Company_Module_Model_Applicant_Student_Abstract { 
// logic related to Student 
}
class Company_Module_Model_Applicant_Dependent extends Mage_Customer_Model_Applicant { 
// inherit Applicant logic and attributes 
}

In the above scenario, using interfaces would add additional layer of complexity.

[sreichel]

For your given example with Mage_Customer_Model_Customer:

The abstract classes are to separate logic (methods) and make same them accessable and overwritable to child classes. This is something you use, when overwriting methods in extension.

Interfaces have no logic.

The Api\<model>\Data only reflects the database-structure for the base-model (and adds constants that match the column names, for easier access). It makes sure you have methods (or-child-method) that matches database-fields.

Magento2 uses this pattern too for its models - and re-use it for API.