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

Include argument types #3

Merged
merged 1 commit into from
May 22, 2020
Merged

Include argument types #3

merged 1 commit into from
May 22, 2020

Conversation

guil95
Copy link
Contributor

@guil95 guil95 commented May 18, 2020

No description provided.

@mattiasgeniar
Copy link
Owner

Hi @guil95 !

Mixed feelings about this PR, I'd love for PHP to introduce the mixed arguments so it could be int|float.

Reason I didn't typehint the arguments is that it works with both integers & floats, I only mandate the return types. I think I'll change this so validate the variables types inside the function itself, until PHP has support for multiple argument types.

@jdreesen
Copy link

Mixed feelings about this PR, I'd love for PHP to introduce the mixed arguments so it could be int|float.

Fortunately, union types come in PHP 8 :)
https://wiki.php.net/rfc/union_types_v2

@guil95
Copy link
Contributor Author

guil95 commented May 19, 2020

I don't agree with that, because I can define a method as float and the method accepts numbers without a floating point. If I leave the mixed type, it will open a gap for the passage of any type, its library is very good.

@mattiasgeniar
Copy link
Owner

I don't agree with that, because I can define a method as float and the method accepts numbers without a floating point. If I leave the mixed type, it will open a gap for the passage of any type, its library is very good.

You are right, I was (wrongfully) under the assumption that a float as input type in the function would require an explicit casting to float beforehand, but that's not the case.

In that case: this is perfect! :-)

Thank you for the PR!

@mattiasgeniar mattiasgeniar merged commit 905f8ca into mattiasgeniar:master May 22, 2020
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