Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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 warningThere 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.
@pquerner mmmm interesting, then your version is better, @kiatng?
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:
$result = $array['a'] ?? []
is the same as$result = isset($array['a']) ? $array['a'] : []
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 statementarray_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.