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

[10.x] Create EmptyArgument class #44042

Closed
wants to merge 2 commits into from

Conversation

browner12
Copy link
Contributor

null can have different meanings when used in different contexts. in a lot of places in the framework, it is used to mean "ignore this parameter when no argument is passed, and execute different behavior than if a value had been passed". for the most part this is fine, but it limits us slightly by not be able to check for a desired null value. this new empty marker class allows us to specifically designate we wish to treat the method call as if the argument was not passed, without conflating the use of null.

this must go to master because it requires a new feature of PHP 8.1 (https://www.php.net/manual/en/functions.arguments.php)

credit to @jasonmccreary for sparking this idea in #43923

there are a lot of places in the framework this would be useful, but I limited this PR to just the one usage change. if this gets merged, I'd PR some more of those changes in.

`null` can have different meanings when used in different contexts. in a lot of places in the framework, it is used to mean "ignore this parameter when no argument is passed, and execute different behavior than if a value had been passed". for the most part this is fine, but it limits us slightly by not be able to check for a desired `null` value. this new empty marker class allows us to specifically designate we wish to treat the method call as if the argument was not passed, without conflating the use of `null`.
@taylorotwell
Copy link
Member

Hmm - not totally sold on this one yet 😄 We can detect if null was actually passed vs. "empty argument" using func_num_args()... we do that in a few places in the framework.

@jasonmccreary
Copy link
Contributor

jasonmccreary commented Sep 8, 2022

@browner12 cool solution. I actually toyed around with func_get_args() after our conversation and realized it can be used. Works for named arguments too. 👍

@browner12
Copy link
Contributor Author

Thanks for the heads-up on func_num_args().

I'd offer two counterpoints.

  • By using func_num_args() we are implicitly assuming what parameters have been passed in. We are dependent on the method signature, and if the signature were to change (added parameters, removed parameters, order of parameters changed), we'd have to make the necessary adjustments in the method body. By using this new Object, we could be explicit about which parameter we are checking, and any changes to the method signature would not necessitate adjustments in the body. Granted, a lot of these method signatures are unlikely to change, but it'd still be "set it and forget it" if we made the switch.
  • In regards to the named arguments, I'd actually say as we start to use them more and more, that's a good reason to stop relying on func_num_args(). Again, the use of func_num_args() implicitly assumes which parameters have been passed in based on the method signature order. Take a look at the following code snippet (https://3v4l.org/O14UU#v8.1.10). Due to the use of named arguments, now we truly don't know which arguments are being passed in, and cannot rely on func_num_args() at all to infer that information.

@jasonmccreary
Copy link
Contributor

@browner12, I mistyped, I meant func_get_args() - which does preserve order allowing you to determine what was actually passed in.

@browner12
Copy link
Contributor Author

Thanks for the clarification. I think the point still stands, as @taylorotwell is referencing the use of func_num_args() throughout the framework.

Also, I believe you'd run into the same implicit issues with func_get_args() since it returns a numerically indexed array, not one keyed by the parameter names. Updated snippet: https://3v4l.org/FJDEY#v8.1.10

@jasonmccreary
Copy link
Contributor

@browner12, sure. I'm only pointing out that it can be done with func_get_args() currently and not require PHP 8.1. Not necessarily that's how I'd do it. 😉

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.

3 participants