feat(range): add step parameter to range function #1257
Conversation
- Add optional step parameter to range function (similar to lodash) - Support positive and negative step values - Maintain backward compatibility with existing range(start, end) signature - Add tests for step functionality
|
|
✅ Deploy Preview for trusting-lumiere-9c7fad ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
There was a problem hiding this comment.
Hey and thanks for sending this PR out. This is a cool change and I would love to add it to the library.
My biggest concern at the moment though is that we have a mismatch between the data-first and the data-last signatures. I understand that this might have been due to the fact that in the way this function is currently written there's no way to differentiate a data-first invocation without a step param and a data-last invocation with one (e.g. they are both range(number, number)).
I think the better API here would be to expand the type for the second param (which is currently used for end) to support both cases by typing it as number | RangeOptions where:
type RangeOptions = {
readonly end: number;
readonly step?: number;
}This would allow us to use the regular purry function for purrying without any modifications to it's regular flow, and only add a single line to the implementation itself to differentiate between being provided an options object or a primitive number. e.g.
function rangeImplementation(
start: number,
endOrOptions: number | RangeOptions,
): number[] {
const { end, step = 1 } = typeof endOrOptions === "number" ? { end: endOrOptions } : endOrOptions;
...
}| test("range with step 2", () => { | ||
| expect(range(0, 10, 2)).toStrictEqual([0, 2, 4, 6, 8]); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
I'd like to have some additional tests:
- step=0.
- step is non-integer (e.g.
2.5). - step is in wrong direction to start and stop (e.g.
range(1,10,-1))
| step: number, | ||
| ): number[] { | ||
| if (step === 0) { | ||
| throw new Error("Step cannot be zero"); |
There was a problem hiding this comment.
The error type here should be RangeError.
| throw new Error("Step cannot be zero"); | |
| throw new RangeError(`range: start=${start} and step=${step} would result in an infinite range!`); |
| end: number, | ||
| step: number, | ||
| ): number[] { | ||
| if (step === 0) { |
There was a problem hiding this comment.
Because of JavaScript's floating-point arithmetics, the more accurate test here would be to check if steps cause a progression, regardless of it being 0, e.g.
| if (step === 0) { | |
| if (start + step === start) { |
| if (step > 0) { | ||
| for (let i = start; i < end; i += step) { | ||
| ret.push(i); | ||
| } | ||
| } else { | ||
| for (let i = start; i > end; i += step) { | ||
| ret.push(i); | ||
| } | ||
| } |
There was a problem hiding this comment.
We can be more concise here:
| if (step > 0) { | |
| for (let i = start; i < end; i += step) { | |
| ret.push(i); | |
| } | |
| } else { | |
| for (let i = start; i > end; i += step) { | |
| ret.push(i); | |
| } | |
| } | |
| for (let i = start; step > 0 ? i < end : i > end; i += step) { | |
| ret.push(i); | |
| } |
Summary
Adds an optional
stepparameter to therangefunction, similar to lodash's implementation.Changes
range(start, end, step)signaturerange(start, end)usageExamples
R.range(1, 20, 5) // => [1, 6, 11, 16]
R.range(20, 1, -5) // => [20, 15, 10, 5]
R.range(0, 10, 2) // => [0, 2, 4, 6, 8]
Checklist
src/index.ts(not needed - existing function)docs/src/content/mapping(already exists)