-
Notifications
You must be signed in to change notification settings - Fork 0
Remove calls to Str::contains and Arr::get when not necessary
#27
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -155,7 +155,7 @@ public function getAttribute($key) | |
| } | ||
|
|
||
| // Dot notation support. | ||
| if (Str::contains($key, '.') && Arr::has($this->attributes, $key)) { | ||
| if (str_contains($key, '.') && Arr::has($this->attributes, $key)) { | ||
|
Owner
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Based on https://laravel.com/docs/10.x/helpers#method-str-contains, am I correct to assume that Is it feasible to use scalar type hints for the |
||
| return $this->getAttributeValue($key); | ||
| } | ||
|
|
||
|
|
@@ -177,7 +177,7 @@ public function getAttribute($key) | |
| protected function getAttributeFromArray($key) | ||
| { | ||
| // Support keys in dot notation. | ||
| if (Str::contains($key, '.')) { | ||
| if (str_contains($key, '.')) { | ||
| return Arr::get($this->attributes, $key); | ||
| } | ||
|
|
||
|
|
@@ -195,7 +195,7 @@ public function setAttribute($key, $value) | |
|
|
||
| $value = $builder->convertKey($value); | ||
| } // Support keys in dot notation. | ||
| elseif (Str::contains($key, '.')) { | ||
| elseif (str_contains($key, '.')) { | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Where does this package require PHP 8+? I don't see it in Hhappy to defer to a subsequent PHPORM ticket.
Owner
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It requires |
||
| // Store to a temporary key, then move data to the actual key | ||
| $uniqueKey = uniqid($key); | ||
| parent::setAttribute($uniqueKey, $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.
This is exactly what was done in
Add::get. https://github.com/laravel/framework/blob/256f4974a09e24170ceeeb9e573651fd5e1c703e/src/Illuminate/Collections/Arr.php#L322-L324There 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.
Noted that we're not using dot notation here (per
Arr::get()docs), so these changes seem sensible.