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

feat: Add Number::pairs #51904

Merged
merged 2 commits into from
Jun 27, 2024
Merged

feat: Add Number::pairs #51904

merged 2 commits into from
Jun 27, 2024

Conversation

hotmeteor
Copy link
Contributor

The Number::pairs() method provides the ability to "split" a number into pairs of min/max values. It's sort of like sliding, except the method will determine the values for you.

The method can create pairs of integers or floats.

Examples:

$output = Number::pairs(25, 10);

// [[1, 10], [11, 20], [21, 25]]

$output = Number::pairs(25, 10, 0);

// [[0, 10], [10, 20], [20, 25]]

$output = Number::pairs(10, 2.5, 0)

// [[0, 2.5], [2.5, 5.0], [5.0, 7.5], [7.5, 10.0]]

Thanks!

@ermesonqueiroz
Copy link

Hi @hotmeteor. Thank you for your pull request. I appreciate the effort. However, I believe it would be more consistent with programming language standards to set the default value of the third parameter to 0. What do you think?

@hotmeteor
Copy link
Contributor Author

Sure, if you think so. I'm not sure what standards to compare it to though? I think from a UX perspective I'd expect it to behave as I have it. But again, if the preference is to start with zero that's fine by me.

Copy link

@ermesonqueiroz ermesonqueiroz left a comment

Choose a reason for hiding this comment

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

LGTM

@ahinkle
Copy link
Contributor

ahinkle commented Jun 26, 2024

Thanks for sending in this PR. Seems very niche to me. What's your use-case?

@hotmeteor
Copy link
Contributor Author

@ahinkle Generating values for batches, similar to what's in the docs: https://laravel.com/docs/11.x/queues#dispatching-batches

@taylorotwell taylorotwell merged commit d130cc2 into laravel:11.x Jun 27, 2024
27 of 28 checks passed
@hotmeteor hotmeteor deleted the number-pairs branch June 27, 2024 19:06
@christophrumpel
Copy link
Contributor

Nice one 👍 What is the reason to call the first argument $to? @hotmeteor

@hotmeteor
Copy link
Contributor Author

hotmeteor commented Jul 1, 2024

It represents the number you're pairing up to, though I'm sure there's other names it could have been given.

I guess I thought like:

Pair "by" 5, up "to" 100, "offset" by 1.

Changes I could see:

  • using a zero-based offset as a default (as suggested elsewhere) to match other method behaviors
  • using better argument names

@christophrumpel

@christophrumpel
Copy link
Contributor

Thanks, I'm not complaining just wanted to know more about the decision before I make a video about the feature since it wasn't that clear to me. But I get your points 👍

@hotmeteor
Copy link
Contributor Author

🙇 Oh cool! Ya, naming is hard and Laravel naming is always top-notch so I just want to make sure it's up to standards 😅

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