Details
-
Improvement
-
Status: Open
-
Normal
-
Resolution: Unresolved
-
None
Description
As it has been remarked in CASSANDRA-9462 and other tickets, the API of AbstractBounds is pretty messy. In particular, it's not terribly consistent nor clear on it's handling of wrapping ranges. It also doesn't make it easily identifiable if an AbstractBounds can be wrapping or not, and there is a lot of place where the code assumes it's not but without really checking it, which is error prone. It's also not a very nice API to use (the fact their is 4 different classes that don't even always support the same methods is annoying).
So we should refactor that API. How exactly is up for discussion however.
At the very least we probably want to stick to a single concrete class that know if it's bounds are inclusive or not. But one other thing I'd personally like to explore is to separate ranges that can wrap from the one that cannot in 2 separate classes (which doesn't mean they can't share code, they may even be subtypes). Having 2 separate types would:
- make it obvious what part of the code expect what.
- would probably simplify the actual code: we unwrap stuffs reasonably quickly in the code, so there probably is a lot of operations that we don't care about on wrapping ranges and lots of operations are easier to write if we don't have to deal with wrapping.
- for the non-wrapping class, we could trivially use a different value for the min and max values, which will simplify stuff a lot. It might be harder to do the same for wrapping ranges (especially since a single "wrapping" value is what IPartitioner assumes; of course we can change IPartitioner but I'm not sure blowing the scope of this ticket is a good idea).
As a side note, Guava has a Range. If we do separate wrapping and non-wrapping ranges, we might (emphasis on "might") be able to reuse it for the non-wrapping case, which could be nice (they have a RangeMap in particular that could maybe replace our custom IntervalTree).