Skip to main content
added 1126 characters in body
Source Link
konijn
  • 34.4k
  • 5
  • 71
  • 267

From a short review:

  • Keep the typeof check out of the recursive part, you only need to do this during the first call of the function
  • The fat arrow syntax is good for inline functions, in this case I would highly encourage you to use the function keyword
  • If nis 1, get out of the function after the push, there is no need to execute the remainder
  • I like verbThing for naming function, so perhaps use swapListElements and permutateList permuteList
  • "heap" is not a JavaScript thing, I would not use it in your naming unless you actually created a "Heap" object
  • newArray is not a great name, especially since most of the time it is not actually a new array, but the array that was passed to the function
  • I had to re-read the code that uses j, the variable is not intuitively named, and the code could use a comment there
  • Furthermore, the ternary statements over 3 lines are mind breaking, I would keep them on 1 line

This is my version of Heap's algorithm, with the above taken into account

function swap(list, pos1, pos2){
  const temp = list[pos1];
  list[pos1] = list[pos2];
  list[pos2] = temp;
}

//Implements Heap's permutation algorithm
//https://en.wikipedia.org/wiki/Heap%27s_algorithm
function permute(list) {

  var out = [];
  list = typeof list === 'string' ? list.split('') : list;
  permuteList(list, list.length);

  function permuteList(list, n) {
    var i;

    if (n == 1) {
      out.push(list.join(''));
    } else {
      for (i = 0; i < n - 1; i++) {
        permuteList(list, n - 1);
        if (n % 2) {
          swap(list,0, n - 1);
        } else {
          swap(list,i, n - 1);
        }
      }
      permuteList(list, n - 1);
    }
  }
  return out;
}

console.log(permute('abc'));
console.log(permute('abcdef').length);

From a short review:

  • Keep the typeof check out of the recursive part, you only need to do this during the first call of the function
  • The fat arrow syntax is good for inline functions, in this case I would highly encourage you to use the function keyword
  • If nis 1, get out of the function after the push, there is no need to execute the remainder
  • I like verbThing for naming function, so perhaps use swapListElements and permutateList permuteList
  • "heap" is not a JavaScript thing, I would not use it in your naming unless you actually created a "Heap" object
  • newArray is not a great name, especially since most of the time it is not actually a new array, but the array that was passed to the function
  • I had to re-read the code that uses j, the variable is not intuitively named, and the code could use a comment there
  • Furthermore, the ternary statements over 3 lines are mind breaking, I would keep them on 1 line

From a short review:

  • Keep the typeof check out of the recursive part, you only need to do this during the first call of the function
  • The fat arrow syntax is good for inline functions, in this case I would highly encourage you to use the function keyword
  • If nis 1, get out of the function after the push, there is no need to execute the remainder
  • I like verbThing for naming function, so perhaps use swapListElements and permutateList permuteList
  • "heap" is not a JavaScript thing, I would not use it in your naming unless you actually created a "Heap" object
  • newArray is not a great name, especially since most of the time it is not actually a new array, but the array that was passed to the function
  • I had to re-read the code that uses j, the variable is not intuitively named, and the code could use a comment there
  • Furthermore, the ternary statements over 3 lines are mind breaking, I would keep them on 1 line

This is my version of Heap's algorithm, with the above taken into account

function swap(list, pos1, pos2){
  const temp = list[pos1];
  list[pos1] = list[pos2];
  list[pos2] = temp;
}

//Implements Heap's permutation algorithm
//https://en.wikipedia.org/wiki/Heap%27s_algorithm
function permute(list) {

  var out = [];
  list = typeof list === 'string' ? list.split('') : list;
  permuteList(list, list.length);

  function permuteList(list, n) {
    var i;

    if (n == 1) {
      out.push(list.join(''));
    } else {
      for (i = 0; i < n - 1; i++) {
        permuteList(list, n - 1);
        if (n % 2) {
          swap(list,0, n - 1);
        } else {
          swap(list,i, n - 1);
        }
      }
      permuteList(list, n - 1);
    }
  }
  return out;
}

console.log(permute('abc'));
console.log(permute('abcdef').length);

added 47 characters in body
Source Link
konijn
  • 34.4k
  • 5
  • 71
  • 267

From a short review:

  • Keep the typeof check out of the recursive part, you only need to do this during the first call of the function
  • The fat arrow syntax is good for inline functions, in this case I would highly encourage you to use the function keyword
  • If nis 1, get out of the function after the push, there is no need to execute the remainder
  • I like verbThing for naming function, so perhaps use swapListElements and permutateList permutateListpermuteList
  • "heap" is not a JavaScript thing, I would not use it in your naming unless you actually created a "Heap" object"heap" is not a JavaScript thing, I would not use it in your naming unless you actually created a "Heap" object
  • newArray is not a great name, especially since most of the time it is not actually a new array, but the array that was passed to the function
  • I had to re-read the code that uses j, the variable is not intuitively named, and the code could use a comment there
  • Furthermore, the ternary statements over 3 lines are mind breaking, I would keep them on 1 line

From a short review:

  • Keep the typeof check out of the recursive part, you only need to do this during the first call of the function
  • The fat arrow syntax is good for inline functions, in this case I would highly encourage you to use the function keyword
  • If nis 1, get out of the function after the push, there is no need to execute the remainder
  • I like verbThing for naming function, so perhaps use swapListElements and permutateList
  • "heap" is not a JavaScript thing, I would not use it in your naming unless you actually created a "Heap" object
  • newArray is not a great name, especially since most of the time it is not actually a new array, but the array that was passed to the function
  • I had to re-read the code that uses j, the variable is not intuitively named, and the code could use a comment there
  • Furthermore, the ternary statements over 3 lines are mind breaking, I would keep them on 1 line

From a short review:

  • Keep the typeof check out of the recursive part, you only need to do this during the first call of the function
  • The fat arrow syntax is good for inline functions, in this case I would highly encourage you to use the function keyword
  • If nis 1, get out of the function after the push, there is no need to execute the remainder
  • I like verbThing for naming function, so perhaps use swapListElements and permutateList permuteList
  • "heap" is not a JavaScript thing, I would not use it in your naming unless you actually created a "Heap" object
  • newArray is not a great name, especially since most of the time it is not actually a new array, but the array that was passed to the function
  • I had to re-read the code that uses j, the variable is not intuitively named, and the code could use a comment there
  • Furthermore, the ternary statements over 3 lines are mind breaking, I would keep them on 1 line
Source Link
konijn
  • 34.4k
  • 5
  • 71
  • 267

From a short review:

  • Keep the typeof check out of the recursive part, you only need to do this during the first call of the function
  • The fat arrow syntax is good for inline functions, in this case I would highly encourage you to use the function keyword
  • If nis 1, get out of the function after the push, there is no need to execute the remainder
  • I like verbThing for naming function, so perhaps use swapListElements and permutateList
  • "heap" is not a JavaScript thing, I would not use it in your naming unless you actually created a "Heap" object
  • newArray is not a great name, especially since most of the time it is not actually a new array, but the array that was passed to the function
  • I had to re-read the code that uses j, the variable is not intuitively named, and the code could use a comment there
  • Furthermore, the ternary statements over 3 lines are mind breaking, I would keep them on 1 line