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

[5.3] Add model methods only and except #15748

Closed
wants to merge 2 commits into from
Closed

[5.3] Add model methods only and except #15748

wants to merge 2 commits into from

Conversation

ntzm
Copy link
Contributor

@ntzm ntzm commented Oct 4, 2016

Feels like except might be a bit inefficient because it will load relations even if they are "ignored". Any ideas on how to improve it without replicating attributesToArray?

foreach ($keys as $key) {
$value = $this->getAttribute($key);

if (! is_null($value)) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This might be a little problematic if the database value is actually null.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not just use Arr::only like you do for except?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point. Don't know why I did that. Will update

@taylorotwell
Copy link
Member

Going to hold off on this since I feel like you could just do Arr::only($model->toArray()... manually.

@ntzm
Copy link
Contributor Author

ntzm commented Oct 6, 2016

Fair enough! Thanks for taking the time to review

@ntzm ntzm deleted the model-except-only branch October 6, 2016 20:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants