IBX-9727: Fixed strict types of MultiLanguageName contracts
#599
Conversation
| return isset($this->names[$this->mainLanguageCode]) | ||
| ? $this->names[$this->mainLanguageCode] | ||
| : reset($this->names); | ||
| if (isset($this->mainLanguageCode, $this->names[$this->mainLanguageCode])) { |
There was a problem hiding this comment.
Note
The content-forms bug is fixed by adding $this->mainLanguage to the isset list prior using it
| if (isset($this->mainLanguageCode, $this->names[$this->mainLanguageCode])) { | ||
| return $this->names[$this->mainLanguageCode]; | ||
| } | ||
| $defaultName = reset($this->names); |
There was a problem hiding this comment.
Philosophical question - wouldn't it be better to use array_key_first here since it's PHP 8.3 on main instead of reset?
There was a problem hiding this comment.
Philosophical question - wouldn't it be better to use
array_key_firsthere since it's PHP 8.3 onmaininstead ofreset?
Doesn't array_key_first return an array key, while reset returns a value?
There was a problem hiding this comment.
it does, but we can use this key to get an item from this array instead of reset which alters the pointer
There was a problem hiding this comment.
it does, but we can use this key to get an item from this array instead of reset which alters the pointer
@barw4 So, something like 570f2c0?
I don't mind it that much. We have there though implicit $this->names[null] to $this->names[''] cast in case if the array is empty. However at least I don't need to deal with false value :D
There was a problem hiding this comment.
yes, that's what I had in mind ❤️
Co-Authored-By: Bartek Wajda <barw4@users.noreply.github.com>
|
Related PRs:
ContentTypeCreateStructbuilding after strict types changes content-forms#92Description:
As uncovered by content-forms#92 CI, we need to ensure in
MultiLanguageNameTrait::getNamethat$this->mainLanguageCodeis initialized before accessing that.This presents a good opportunity to add strict types to the
MultiLanguageNamecontracts tied to it.Note: The trait itself was not a very good design (e.g., references
mainLanguageCodedefined elsewhere), but let's tackle one issue at a timeFor QA:
Regression build: 🟢 ibexa/commerce#1354