Sign up ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

Python has by far the best slice implementation I've ever used. As a quick evening project I felt like trying to implement Python's slice construct in a JavaScript function (thinking about sending a pr to a certain library with Python's slice semantics). I came up with the code below, and I'm trying to improve my code. I would love feedback or alternative implementations.

I think I hit all the edge cases, but I only briefly skimmed the Python source, so I could easily be wrong.

/**
 * Implementation of Python's `slice` function... Get a cloned subsequence
 * of an iterable (collection with length property and array like indexs).
 * Will handle both strings and array(likes).
 * 
 * @param {Array|String} collection
 * @param {None|Integer} start First index to include. If negative it will be indicies from end
                             (i.e. -1 is last item). Omit or pass 0/null/undefined for 0.
 * @param {None|Integer} end Last index to include. If negative it will be indicies from end
                             (i.e. -1 is last item). Omit or pass null/undefined for end.
 * @param {None|Intger} step Increments to increase by (non-1 will skip indicies). Negative values
                              will reverse the output.
 * @returns {Array|String} sliced array
 *
 * @example
 * var list = [1, 2, 3, 4, 5]
 * slice(list) // => [1, 2, 3, 4, 5]
 * slice(list, 2) // => [3, 4, 5]
 * slice(list, 2, 4) // => [3, 4]
 * slice(list, -2) // => [4, 5]
 * slice(list, null, -1) // => [1, 2, 3, 4]
 * slice(list, null, null, 2) // => [1, 3, 5]
 * slice(list, null, null, -2) // => [5, 3, 1]
 * slice("kids a devil I tell ya", 7, -10, -1) // => "lived"
 */
function slice(collection, start, end, step) {
    var length = collection.length,
        isString = typeof collection == "string", // IE<9 have issues with accessing strings by indicies ("str"[0] === undefined)
        result = [];
    if (isString) {
        collection = collection.split("");
    }
    if (start == null) {
        start = 0;
    } else if (start < 0) {
        start = length + start;
    }
    if (end == null || end > length) {
        end = length;
    } else if (end < 0) {
        end = length + end;
    }
    if (step == null) {
        step = 1;
    } else if (step === 0) {
        throw "Slice step cannot be zero";
    }
    if (step > 0) {
        for (; start < end; start += step) {
            result.push(collection[start]);
        }
    } else {
        for (end -= 1; start <= end; end += step) {
            result.push(collection[end]);
        }
    }
    // Return a string for input strings otherwise an array
    return isString ? result.join("") : result;
}
share|improve this question
    
I had an answer here, but I think it can be reduced to a comment: 1) throw errors and not strings, and 2) that start == null might confuse people. It confused me (and I'm not normally one of the "always use ===" pundits either). – Dagg Jul 17 '14 at 4:38
    
Welcome to javascript. You can put your example into [jasmine](jasmine.github.io) as tests. – neo Jul 17 '14 at 4:54
    
for readability i would personally go for if ( is_null(param) ) but appart from that. I like what you did there – Pinoniq Jul 17 '14 at 8:02
    
@dagg I liked your answer. Esp handling steps – megawac Jul 17 '14 at 12:29
    
Particularly it made me reconsider using push instead of precomputing the length: length = Math.ceil(Math.max(end - start, 0) / Math.abs(step)), result = Array(length); The I can iterate like you! – megawac Jul 17 '14 at 12:44

1 Answer 1

Throwing errors

You should only throw errors, not strings.

throw new Error("Slice step cannot be zero");

Leveraging existing API

What you're doing looks very similar to the existing slice except for the last parameter, step. This could be more compact if you used built-in slice for everything and then apply the step afterward.

function slice(collection, start, end, step) {
    var slice = collection.slice || Array.prototype.slice,
        sliced = slice.call(collection, start, end),
        result, length, i;

    if (!step) {
        return sliced;
    }
    result = [];
    length = sliced.length;
    i = (step > 0) ? 0 : length - 1;
    for (; i < length && i >= 0; i += step) {
        result.push(sliced[i]);
    }
    return typeof collection == "string" ? result.join("") : result;
}

This would require you to pass undefined in the places you have null in your usage examples. If you want to be able to pass null, you can add if (start === null) start = undefined, and the same for end.

share|improve this answer
    
start == null only returns true for undefined and null which I think ~~ the Python NoneType struct, whereas !start returns true for NaN, 0, etc. Good point on the Error. I was trying to avoid calling the native slice for cross environment concerns (ie nodelists) – megawac Jul 17 '14 at 4:13
    
@megawac What's the problem with calling slice on nodelists? Something like [].slice.apply(document.scripts) should work fine everywhere shouldn't it? – Dagg Jul 17 '14 at 4:17
    
Throws an error on IE<=8 IIRC. Still points for nice alternative! – megawac Jul 17 '14 at 4:21
    
Ugh, you're right. And I actually commented on that SO thread you linked years ago, so I must have read it. How quickly we forget... – Dagg Jul 17 '14 at 4:28
1  
@megawac there was no need to, since collection.slice was String.prototype.slice in that case, if I'm understanding you correctly. That basically worked as an alias for "smt".slice(1, 2). Take another look at your perf; in daggTest you test if sliced is a string and if so, try to call sliced.join... – Dagg Jul 18 '14 at 2:21

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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