Model performance improvements #646
Conversation
…micro optimizations: Less array like access of the field name, less function calls - BaseModel::_setFieldData: replaced while-list-each with foreach, reduced calls of isPropertyLoaded
… existing currencies too
| @@ -985,8 +985,15 @@ protected function _update() | |||
| protected function _setFieldData($sFieldName, $sValue, $iDataType = \OxidEsales\Eshop\Core\Field::T_TEXT) | |||
| { | |||
| //preliminary quick check saves 3% of execution time in category lists by avoiding redundant strtolower() call | |||
| if ($sFieldName[2] == 'l' || $sFieldName[2] == 'L' || (isset($sFieldName[16]) && ($sFieldName[16] == 'l' || $sFieldName[16] == 'L'))) { | |||
| if ('oxlongdesc' === strtolower($sFieldName) || 'oxcategories__oxlongdesc' === strtolower($sFieldName)) { | |||
| if (($sFieldNameIndex2 = $sFieldName[2]) === 'l' | |||
There was a problem hiding this comment.
Please don't use hungarian notations anymore. Check our coding styles agreements.
There was a problem hiding this comment.
Please don't make complex conditions even more complex :) Try to improve the readability instead.
There was a problem hiding this comment.
Hungarian: I know, but wanted to keep the method in a consistent style. I changed it :)
Without the assignment within the condition we either need to assign the character before (including isset) or duplicate the conditioned code. I think both options are worse than the maybe-saving of an access of the 16 index, I reverted most of the condition
| $cur = $this->getShopCurrency(); | ||
| $currencies = $this->getCurrencyArray(); | ||
| if (!isset($currencies[$cur])) { | ||
| $this->_oActCurrencyObject = reset($currencies); // reset() returns the first element |
There was a problem hiding this comment.
Please check the coding standards. "_" and "hungarian" notations are not allowed in new code anymore.
There was a problem hiding this comment.
The property already exists, renaming it because of a change of indentation would be too much for the addition of an "if" I think. Are you sure?
There was a problem hiding this comment.
@MaxBoeh, you are right, sorry, haven't noticed. I thought it is some new one for caching. Thanks for clarifying.
Did some (micro) optimizations mainly regarding model loading / data assignment.