1
\$\begingroup\$

I wanted a neat iterator that will always give me neat sequence of step spaced numbers. This is what I came up with, but I am a bit skeptical that I covered all cases.

/**
 * Creates numeric range iterator. The iterator will iterate between start and end.
 * Start may be larger than end. The output will contain both start and end, but no value
 * will appear twice.
 * @param {number} start
 * @param {number} end
 * @param {number} step
 * @yields {number}
 * @returns {IterableIterator<number>}
 */
function* numrange(start = 0, end = 100, step = 1) {
    if (start > end) {
        step = -1 * step;
    }

    let i = start;
    for (; shouldLoop(i); i += step) {
        yield i;
        if (i!=end && !shouldLoop(i + step)) {
            yield end;
        }
    }

    function shouldLoop(ival) {
        return start > end ? ival >= end : ival <= end;
    }
}

Can this be improved?

\$\endgroup\$
2
  • \$\begingroup\$ Does it work well and as intended for negative values passed for end but positive vaules of step? \$\endgroup\$ Oct 11, 2019 at 10:27
  • \$\begingroup\$ Good question. Step must be positive, it is inverted internally if needed. And yeah, it works in that case: [...numrange(3,-5)] -> Array(9) [ 3, 2, 1, 0, -1, -2, -3, -4, -5 ] \$\endgroup\$ Oct 11, 2019 at 10:41

1 Answer 1

1
\$\begingroup\$

Counter intuitive?

Infinite loops.

The function is an iterator so this may not be undesired or potentially fatal for the application. However some inputs would make one wonder about the callers intent.

numrange(0, 100, 0); // will endlessly return 0
numrange(0, 100, -1); // will return 0, -1, -2, ... Infinity
numrange(0, -100, 1); // will return 0, 1, 2, ... Infinity

Not part of sequence.

There are situation where the last value can be out of sequence

numrange(0, 4, 2); // will return 0, 2.5, 4.

Or two values are returned rather than one.

numrange(4, 4, 1); // will return 4, 4.

Names

  • The function name should be numRange to be consistent with JS camelCase convention.
  • The argument ival (should be iVal) in shouldLoop is excessive, you could just have used i.
  • The function name shouldLoop can also be simplified as it expresses a question "should loop?" which can be shortened considering the context of its use to "loop?"

Logic and flow

The function shouldLoop is called 2 times each iteration, there is no need to do this, on top of this there are cases where the first clause of the inner statement is redundant. eg numRange(0, 20, 1.01) BTW use strict inequality !== rather than !=

You can improve efficiency if you removed the inner statement and yielded end after the loop. You would also need to change the return condition inside the function shouldLoop to start > end ? ival > end : ival < end

Each time you iterate you test if start > end This is known from the very start of the function and is thus repeating the same logic for no need. Use the condition to assign one of two functions and avoid the repeated logic.

Rewrite

Thus you can have the same behavior as your original code with

function* numRange(start = 0, end = 100, step = 1) {
    const loop = start > end ? (i) => i > end : (i) => i < end;
    if (start > end) { step = -1 * step }
    for (let i = start; loop(i); i += step) { 
        yield i;
    }
    yield end;
}

Example

  • Fixing what I see as problems (as outlined at the start of the answer)
    • Ensure that step is in the correct direction no matter what its sign.
    • Check if step is 0 and just return the start if so.
    • Check if end is part of the sequence and ignore it if not.
  • Using the var as argument start to count.

.

function* numRange(start = 0, end = 100, step = 1) {
    step = Math.sign(end - start) * Math.abs(step);
    if (step) {
        const loop = step > 0 ? () => start < end : () => end < start;
        while (loop()) {
            yield start;
            start += step;
        }
        if (start !== end) { return }
    }
    yield start; 
}
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.