Skip to content
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

Merged
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion app/code/core/Mage/Eav/Model/Config.php
Original file line number Diff line number Diff line change
Expand Up @@ -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()])
Copy link
Contributor

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?

Copy link
Contributor

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.

Copy link
Contributor

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. :)

Copy link
Contributor

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.

Copy link
Contributor

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

Copy link
Contributor

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.

Copy link
Contributor

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?

Copy link
Contributor Author

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:

  1. The null coalescing operator (??) doc is here. $result = $array['a'] ?? [] is the same as $result = isset($array['a']) ? $array['a'] : []
  2. 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.

? array_keys($this->_entityTypeAttributeIdByCode[$storeId][$entityType->getId()])
: [];
}
}

Expand Down
Loading