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

[11.x] Added Number::isBetween() method #52587

Closed
wants to merge 1 commit into from
Closed

Conversation

soltmar
Copy link

@soltmar soltmar commented Aug 27, 2024

This PR adds new isBetween method to Number class.

Method tests if given number is between given min and max. There is also 4th $inclusive parameter where we can decide wether min and max should be inclusive or not.

I've had this function for years in my helpers.php file and though that since we have Number class for a while now it may be worth to include it there so others can benefit from it.

I'm wondering why nobody came up with this simple method before. Maybe there was a reason, if this is the case could you point me to the PR so I can see explanation please ?

I know this is very basic functionality but believe this will improve code readability and see it being used a lot by Laravel community.

Thanks

@soltmar soltmar force-pushed the 11.x branch 2 times, most recently from 0577f85 to 0e7630a Compare August 27, 2024 12:10
@soltmar soltmar changed the title Added Number::isBetween() method [11.x] Added Number::isBetween() method Aug 27, 2024
@taylorotwell
Copy link
Member

Thanks for your pull request to Laravel!

Unfortunately, I'm going to delay merging this code for now. To preserve our ability to adequately maintain the framework, we need to be very careful regarding the amount of code we include.

If applicable, please consider releasing your code as a package so that the community can still take advantage of your contributions!

@soltmar
Copy link
Author

soltmar commented Aug 27, 2024

It doesn't make sense to create a package from what I've done here. It's not a big change / addition and Number class seem like a perfect place for this.

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