Skip to main content
incorporated `TypeError` suggestion
Source Link
amon
  • 12.6k
  • 36
  • 67

Your code seems to be correct, and is written in a good style. However, both the interface of your code as well as the algorithm used can be improved.

You currently have a global variable permutations which also contains the output. This variable will have to be cleared every time before doPerm is invoked. Instead, wrap doPerm in another function that provides initialization and input validation:

function perms(data) {
    if (!(data instanceof Array)) {
        throw new TypeError("input data must be an Array";Array");
    }

    data = data.slice();  // make a copy
    var permutations = [],
        stack = [];

    function doPerm() {
        if (data.length == 0) {
            permutations.push(stack.slice());
        }
        for (var i = 0; i < data.length; i++) {
            var x = data.splice(i, 1);
            stack.push(x);
            doPerm();
            stack.pop();
            data.splice(i, 0, x);
        }
    }

    doPerm();
    return permutations;
}

var input = "abcd".split('');
var result = perms(input);
for (var i = 0; i < result.length; i++) {
    result[i] = result[i].join('');
}
console.log(result);

Note that this function will operate on arrays of any type and does not special-case strings.

Your algorithm currently relies on splicing elements into and out of the input array. This is somewhat expensive on larger data sets, and it would be better to generate the permutations by swapping. Heap's Algorithm does just this.

Your code seems to be correct, and is written in a good style. However, both the interface of your code as well as the algorithm used can be improved.

You currently have a global variable permutations which also contains the output. This variable will have to be cleared every time before doPerm is invoked. Instead, wrap doPerm in another function that provides initialization and input validation:

function perms(data) {
    if (!(data instanceof Array)) {
      throw "input data must be an Array";
    }

    data = data.slice();  // make a copy
    var permutations = [],
        stack = [];

    function doPerm() {
        if (data.length == 0) {
            permutations.push(stack.slice());
        }
        for (var i = 0; i < data.length; i++) {
            var x = data.splice(i, 1);
            stack.push(x);
            doPerm();
            stack.pop();
            data.splice(i, 0, x);
        }
    }

    doPerm();
    return permutations;
}

var input = "abcd".split('');
var result = perms(input);
for (var i = 0; i < result.length; i++) {
    result[i] = result[i].join('');
}
console.log(result);

Note that this function will operate on arrays of any type and does not special-case strings.

Your algorithm currently relies on splicing elements into and out of the input array. This is somewhat expensive on larger data sets, and it would be better to generate the permutations by swapping. Heap's Algorithm does just this.

Your code seems to be correct, and is written in a good style. However, both the interface of your code as well as the algorithm used can be improved.

You currently have a global variable permutations which also contains the output. This variable will have to be cleared every time before doPerm is invoked. Instead, wrap doPerm in another function that provides initialization and input validation:

function perms(data) {
    if (!(data instanceof Array)) {
        throw new TypeError("input data must be an Array");
    }

    data = data.slice();  // make a copy
    var permutations = [],
        stack = [];

    function doPerm() {
        if (data.length == 0) {
            permutations.push(stack.slice());
        }
        for (var i = 0; i < data.length; i++) {
            var x = data.splice(i, 1);
            stack.push(x);
            doPerm();
            stack.pop();
            data.splice(i, 0, x);
        }
    }

    doPerm();
    return permutations;
}

var input = "abcd".split('');
var result = perms(input);
for (var i = 0; i < result.length; i++) {
    result[i] = result[i].join('');
}
console.log(result);

Note that this function will operate on arrays of any type and does not special-case strings.

Your algorithm currently relies on splicing elements into and out of the input array. This is somewhat expensive on larger data sets, and it would be better to generate the permutations by swapping. Heap's Algorithm does just this.

Source Link
amon
  • 12.6k
  • 36
  • 67

Your code seems to be correct, and is written in a good style. However, both the interface of your code as well as the algorithm used can be improved.

You currently have a global variable permutations which also contains the output. This variable will have to be cleared every time before doPerm is invoked. Instead, wrap doPerm in another function that provides initialization and input validation:

function perms(data) {
    if (!(data instanceof Array)) {
      throw "input data must be an Array";
    }

    data = data.slice();  // make a copy
    var permutations = [],
        stack = [];

    function doPerm() {
        if (data.length == 0) {
            permutations.push(stack.slice());
        }
        for (var i = 0; i < data.length; i++) {
            var x = data.splice(i, 1);
            stack.push(x);
            doPerm();
            stack.pop();
            data.splice(i, 0, x);
        }
    }

    doPerm();
    return permutations;
}

var input = "abcd".split('');
var result = perms(input);
for (var i = 0; i < result.length; i++) {
    result[i] = result[i].join('');
}
console.log(result);

Note that this function will operate on arrays of any type and does not special-case strings.

Your algorithm currently relies on splicing elements into and out of the input array. This is somewhat expensive on larger data sets, and it would be better to generate the permutations by swapping. Heap's Algorithm does just this.