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

ordering of parameters makes isBetween a non-starter #1

Open
michaelficarra opened this issue Jul 12, 2022 · 5 comments
Open

ordering of parameters makes isBetween a non-starter #1

michaelficarra opened this issue Jul 12, 2022 · 5 comments

Comments

@michaelficarra
Copy link

Nobody is ever going to remember the order of parameters for isBetween. Without looking, if I had to guess at an order, it would be "lower bound", "upper bound", then the number being tested. Actually, I lied because I would expect the bounds to be given in either order ("between" works both ways in English). I don't see a way this could ever be resolved, and that's not even touching upon the inclusivity question of the bounds. I suggest you abandon isBetween and go with the alternative suggestion of an isIncreasing (or isNotDecreasing) and isStrictlyIncreasing function.

@ljharb
Copy link

ljharb commented Jul 12, 2022

I would expect (target, min, max), or (min, target, max) as if min <= target <= max - never target being last.

@ljharb
Copy link

ljharb commented Jul 12, 2022

One possibility would be Number.prototype.isBetween(min, max), though.

@michaelficarra
Copy link
Author

@ljharb Then you still have to resolve the issue with "between" implying no bounds order and the inclusivity issue. It's not worth going that route.

@ljharb
Copy link

ljharb commented Jul 12, 2022

True. "is between" to me implies it's exclusive on both ends, though, and "order" would be LTR.

@js-choi
Copy link
Owner

js-choi commented Jul 12, 2022

Thank you for the feedback; I am ready to drop isBetween and switch to the isIncreasing approach. I’d still like to get a quick temperate check at plenary if this achieves Stage 1.

The isStrictlyIncreasing name (versus having isIncreasing take a boolean) is a good idea, and I’ll add it as an option later.

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

No branches or pull requests

3 participants