-
-
Notifications
You must be signed in to change notification settings - Fork 436
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fixed undefined array key in Mage/Eav/Model/Config.php #4059
Fixed undefined array key in Mage/Eav/Model/Config.php #4059
Conversation
@@ -571,7 +571,9 @@ public function getEntityAttributeCodes($entityType, $object = null) | |||
} | |||
return $attributeCodes; | |||
} else { | |||
return array_keys($this->_entityTypeAttributeIdByCode[$storeId][$entityType->getId()]); | |||
return isset($this->_entityTypeAttributeIdByCode[$storeId][$entityType->getId()]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wouldnt return array_keys($this->_entityTypeAttributeIdByCode[$storeId][$entityType->getId()] ?? [])
do the same?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changing array_keys to isset changes the return of the function which is supposed to return attribute codes, not a boolean value.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@davidhiendl It doesnt return boolean. I think you read that wrong. :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah that code comment snippet function did cut of mid-change... fabulous.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@pquerner I think array_keys($this->_entityTypeAttributeIdByCode[$storeId][$entityType->getId()] ?? [])
would still trigged the undefined array index warning
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@fballiano https://onlinephp.io/c/eebcc nope, unless I did something wrong.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
array_keys($this->_entityTypeAttributeIdByCode[$storeId][$entityType->getId()] ?? [])
works but not necessary better.
My break down:
- The null coalescing operator (??) doc is here.
$result = $array['a'] ?? []
is the same as$result = isset($array['a']) ? $array['a'] : []
- after the above, we have
array_kyes($result)
, an additional step if key 'a' is not set.
This isset($array['a']) ? array_keys($array['a']) : []
works slightly better if key 'a' is not set. However, this statement array_keys($array['a'] ?? [])
is shorter.
I vote for the lengthy one: it has clearer intent and works slightly better if the key is not set. Having said that, I am fine either one.
Description (*)
A ban person in OM community asked me to create this PR. I reviewed it and to me. this is a valid PR.
Related Pull Requests
PR #4036