guava
guava copied to clipboard
Add a `constrain` method to Range
com.google.common.primitives.Ints
already has the very useful static int constrainToRange(int value, int min, int max)
. It would be nice to add this as a method to the Range
class.
Two advantages:
- This would be generically available to any
Comparable
type. - It just reads better- compare
Range.closed(3,9).constrain(4)
vsInts.constrainToRange(3,4,9)
. In the latter case it's not immediately clear which parameter is the input and which are the bounds, but in the former it's super obvious.
One problem is; what should the output of Range.greaterThan(3).constrain(1)
be?
- On one hand, it seems like
myRange.contains(myRange.constrain(x))
should always betrue
, so maybe we should throw anIllegalArgumentException
if the exceeded bound is open. - On the other hand,
(3...5].constrain(1) = 3
doesn't seem entirely unreasonable either.
@cpovirk @nakulj Hello. I would like to work on this issue. But This is my first time contributing to an open-source project. Therefore, I need some guidance. Could you please give me more information about this issue?
I do think it's important that a constrain()/clamp() method never return a value the range doesn't contain. And that means we can't really do this without requiring a DiscreteDomain, and that's really awkward. Even ranges canonicalized using a DD still have this problem, since it canonicalizes to half-open.
Or it would be a method that works sometimes, which is also awkward.
Perhaps ContiguousSet could support this, though. Unfortunately, ContiguousSet is really what most people want when they reach for Range, but Range got the prime real estate.
ContiguousSet is really what most people want
to be fair that's not really the first name someone would think to google for when thinking about libraries that might solve the problem of dealing with 'ranges of values'.
Perhaps a class called 'DiscreteRange' might have represented the concept better.
Putting clamp
in ContiguousSet
would be fine, but just look at what the resultant code might look like-
ContiguousSet.create(Range.closed(lower,upper), bigIntegers()).clamp(input);
Which is just... so much code.
I think a sometimes-working version that was in Range would be fine. As long as the limitations were documented, it would be fine imo.
Yeah, you've articulated the exact reason I used the word "unfortunately". Yes, it is unfortunate. It just doesn't necessarily mean that adding a bunch of DiscreteDomain-accepting methods to Range is the answer though. The sometimes-working-method idea is problematic in a bunch of ways too. I don't know which way this will go.
I would like to work on this issue if it's not been implemented yet.
@u6873285 I don't think it's a matter of implementation, but design. I don't own the repo but I suggest you start by adding your proposed API as a comment on this issue for discussion. If the owners approve, then you can create a PR.
I'm afraid the prospects for this are not good, given the messy situation with DiscreteDomain/ContiguousSet. If there were to be IntRange, LongRange, etc. classes, then clamp/constrain would work fine there. But I don't see us doing that now.