-
-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
ref: RangeMap
class
#83259
Merged
Merged
ref: RangeMap
class
#83259
Changes from all commits
Commits
Show all changes
13 commits
Select commit
Hold shift + click to select a range
eecc605
Implement ladder class
gggritso 896fbb2
Implement min and max
gggritso 74cca64
Replace `GranularityLadder` internals
gggritso b92a13b
Provide generic types
gggritso 0a70a7e
Add types
gggritso fa8c2d4
Add doc example
gggritso 8020b30
Fix spec for min and max
gggritso 02f8441
Correct the logic
gggritso 7f2beba
Merge branch 'master' into ref/ggg/ladder-class
gggritso 2351c1a
New API behaviour
gggritso e6ae9bf
Rename to `RangeMap`
gggritso 69eb20a
Revert refactor
gggritso 0061369
Improve error handling
gggritso File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,77 @@ | ||
import {RangeMap} from './rangeMap'; | ||
|
||
describe('RangeMap', () => { | ||
test('matches an integer range', () => { | ||
const ladder = new RangeMap([ | ||
{min: 0, max: 10, value: 'first'}, | ||
{min: 10, max: 20, value: 'second'}, | ||
{min: 20, max: 50, value: 'third'}, | ||
]); | ||
|
||
expect(ladder.get(-10)).toBeUndefined(); | ||
expect(ladder.get(5)).toBe('first'); | ||
expect(ladder.get(15)).toBe('second'); | ||
expect(ladder.get(25)).toBe('third'); | ||
expect(ladder.get(58)).toBeUndefined(); | ||
}); | ||
|
||
test('must provide at least one range', () => { | ||
// @ts-ignore | ||
expect(() => new RangeMap([])).toThrow(); | ||
}); | ||
|
||
test('cannot have overlapping ranges', () => { | ||
expect( | ||
() => | ||
new RangeMap([ | ||
{min: 0, max: 10, value: 'first'}, | ||
{min: 10, max: 20, value: 'second'}, | ||
{min: 15, max: 25, value: 'third'}, | ||
]) | ||
).toThrow(); | ||
}); | ||
|
||
test('may have range gaps', () => { | ||
const ladder = new RangeMap([ | ||
{min: 0, max: 10, value: 'first'}, | ||
{min: 10, max: 20, value: 'second'}, | ||
{min: 50, max: 100, value: 'third'}, | ||
]); | ||
|
||
expect(ladder.get(25)).toBeUndefined(); | ||
}); | ||
|
||
test('range minimum is inclusive', () => { | ||
const ladder = new RangeMap([ | ||
{min: 0, max: 10, value: 'first'}, | ||
{min: 10, max: 20, value: 'second'}, | ||
{min: 20, max: 50, value: 'third'}, | ||
]); | ||
|
||
expect(ladder.get(0)).toBe('first'); | ||
expect(ladder.get(10)).toBe('second'); | ||
}); | ||
|
||
test('provides the min and max value', () => { | ||
const ladder = new RangeMap([ | ||
{min: 0, max: 10, value: 'first'}, | ||
{min: 10, max: 20, value: 'second'}, | ||
{min: 20, max: 50, value: 'third'}, | ||
]); | ||
|
||
expect(ladder.min).toBe('first'); | ||
expect(ladder.max).toBe('third'); | ||
}); | ||
|
||
test('enforces type', () => { | ||
type Salutation = 'Hello' | 'Hi'; | ||
|
||
const ladder = new RangeMap([ | ||
{min: 0, max: 10, value: 'Hello' as Salutation}, | ||
{min: 10, max: 20, value: 'Hi' as Salutation}, | ||
]); | ||
|
||
const salutation = ladder.get(0); | ||
expect(salutation === 'Hi').toBeFalsy(); | ||
}); | ||
}); |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,69 @@ | ||
import orderBy from 'lodash/orderBy'; | ||
|
||
export type Range<T> = { | ||
max: number; | ||
min: number; | ||
value: T; | ||
}; | ||
|
||
/** | ||
* Maps a set of integer ranges to the corresponding values. | ||
* | ||
* @example | ||
* ```javascript | ||
* // Map plan price to support tier | ||
* const teamSizeToSupportTierMap = new RangeMap([ | ||
* {min: 0, max: 10, value: 'basic'}, | ||
* {min: 10, max: 30, value: 'premium'}, | ||
* {min: 30, max: 50, value: 'ultra-premium'}, | ||
* ]); | ||
* | ||
* const quota = quotaToTierMap.get(0); // Tier for small teams is "basic" | ||
* const quota = quotaToTierMap.get(12); // Tier for a 10-30 person team is "premium" | ||
* ``` | ||
*/ | ||
export class RangeMap<T> { | ||
JonasBa marked this conversation as resolved.
Show resolved
Hide resolved
|
||
ranges: Range<T>[]; | ||
|
||
constructor(ranges: Range<T>[]) { | ||
// Filter out sparse array slots just in case | ||
const filteredRanges = ranges.filter(Boolean); | ||
JonasBa marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
if (filteredRanges.length === 0) { | ||
throw new Error('No ranges provided'); | ||
} | ||
|
||
const sortedRanges = orderBy(filteredRanges, range => range.min, 'asc'); | ||
|
||
for (let i = 1; i < sortedRanges.length; i += 1) { | ||
const previousRange = sortedRanges[i - 1]!; | ||
const range = sortedRanges[i]!; | ||
|
||
if (previousRange.max > range.min) { | ||
throw new Error( | ||
`${rangeToString(range)} overlaps with ${rangeToString(previousRange)}` | ||
); | ||
} | ||
} | ||
|
||
this.ranges = sortedRanges; | ||
} | ||
|
||
get(value: number) { | ||
JonasBa marked this conversation as resolved.
Show resolved
Hide resolved
|
||
return this.ranges.find(r => { | ||
return value >= r.min && value < r.max; | ||
})?.value; | ||
} | ||
|
||
get min() { | ||
return this.ranges.at(0)!.value; | ||
} | ||
|
||
get max() { | ||
return this.ranges.at(-1)!.value; | ||
} | ||
} | ||
|
||
function rangeToString(range: Range<any>): string { | ||
return `Range min:${range.min}, max:${range.max}, value:${range.value}`; | ||
} |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How about something like this to indicate that values are inclusive?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm a little wary of that because it only works for integers, and the class doesn't do anything to enforce integer-only use. Should it? It seems like there's a use-case for matching ranges to floats though I don't see it in the code right now.
Another option is maybe to make the boundary matching explicit, i.e., allow configuring which boundary is inclusive, and have it be
min
by default