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

[7.x] Add isNotFilled() method to Request #33732

Merged
merged 2 commits into from
Aug 5, 2020
Merged

[7.x] Add isNotFilled() method to Request #33732

merged 2 commits into from
Aug 5, 2020

Conversation

KennedyTedesco
Copy link
Contributor

@KennedyTedesco KennedyTedesco commented Aug 2, 2020

Very often, I use ! $request->filled(), and it's ok, I can live with it. But I would love to have an empty() method. So, instead of:

if (! $request->filled('recaptcha')) {
    // TODO
}

Would be possible to use:

if ($request->empty('recaptcha')) {
    // TODO
}

The empty() method here is just the opposite of filled() (in the sense of behavior) to maintain the cognitive consistency when using one or another.

About the name, if empty() isn't that good, we can choose another one, but I prefer the first one.

Ps: I know I can use a macro if this gets rejected. 😁

@derekmd
Copy link
Contributor

derekmd commented Aug 2, 2020

In other parts of the framework, "blank" is used as the opposite of filled: https://laravel.com/docs/7.x/helpers#method-blank

PHP's empty() function returns true for undefined symbols or falsey values so:

empty('0');
// true

request()->replace(['foo' => '0'])->empty('foo');
// false

Since empty() behavior differs from the opposite of filled(), blank() is used instead.

(Ruby on Rails also uses blank: https://api.rubyonrails.org/classes/Object.html#method-i-blank-3F)

@GrahamCampbell
Copy link
Member

PHP's empty() function returns true for undefined symbols or falsey values so:

I don't think we should try to emulate PHP's empty function. That behaviour is usually not what people intended.

@KennedyTedesco
Copy link
Contributor Author

KennedyTedesco commented Aug 2, 2020

We already don't emulate !empty() in the filled() method. I've kept the same behavior.

About the name, I'm open to change it, if requested. My suggestions are: empty() or notFilled().

@shalvah
Copy link
Contributor

shalvah commented Aug 3, 2020

@GrahamCampbell I think @derekmd was suggesting w use the name blank(), not that we emulate empty()'s behaviour. He was just giving some backstory into the reason for the name.

@taylorotwell
Copy link
Member

If we rename to blank there will be a subtle difference between the blank helper and this method. Namely, empty arrays are considered blank by the blank helper but would not be considered blank by this method.

@KennedyTedesco
Copy link
Contributor Author

The same way, filled() considers empty arrays as true:

request()->replace(['foo' => []])->filled('foo'); // true

My goal here is to find a way to negate the filled().

@taylorotwell
Copy link
Member

taylorotwell commented Aug 4, 2020

So far I'm just not happy with the naming options. We could use isNotFilled to be super explicit. Similar to Collection's isNotEmpty. The empty and blank options both have some drawbacks I'm not comfortable with.

@KennedyTedesco KennedyTedesco changed the title [7.x] Add empty() method to Request [7.x] Add isNotFilled() method to Request Aug 4, 2020
@KennedyTedesco
Copy link
Contributor Author

isNotFilled() looks pretty good to me, super explicit. I've changed the method name.

@taylorotwell taylorotwell merged commit 71ebdca into laravel:7.x Aug 5, 2020
@KennedyTedesco KennedyTedesco deleted the 7.0-request branch August 5, 2020 16:21
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.

5 participants