My selection sort implementation

Question

please review my code on selection sort algorithm:

selectionSort();
function selectionSort(){
 var numArray = [4,34,2,5,12];
 var MIN = 0;
 var totalNum = numArray.length,
 temp = 0,
 next_position = 0
 for(var i = 0; i < totalNum; i++){
 MIN = numArray[i];
 next_position = i + 1;
 for(var j = next_position; j < totalNum; j++){
 if(numArray[j] < MIN){
 temp = MIN;
 MIN = numArray[j];
 numArray[j] = temp;
 }
 }
 numArray[i] = MIN; 
 }
 console.log(numArray);
}

After running, the output is:

[ 2, 4, 5, 12, 34 ]

• Zorgatone: A function like that won't be re-usable. You are declaring the array to be sorted inside the function, instead of accepting a parameter.
• Zorgatone: Also, a style tip: you're missing a semicolon after next_position = 0
• Zorgatone: Also about "doing this efficiently", selection-sort isn't that efficient: in terms of time complexity it is O(x²)
• Zorgatone: usually Array.prototype.sort is enough for most use-cases (in terms of efficiency), without having to code your own sorting function, which is potentially slower, and prone to errors. If you don't want to use my_array.sort() you could find a javascript library to do it, that should be well-code, more efficient, and without bugs (also saving you some time)
• Denis: @Zorgatone It's probably just a training exercise.

Answer 1

Here's what I would add if I were doing this as a peer review.

Nested For Loops:

Nested for loops are hard to read. As an alternative, split the nested loop into a separate, aptly named function, if possible. Perhaps a function that returns the next item to be swapped with? You would take your nested for loop and move it to a function that would return what is currently temp if it is there. Then, if there was a return value, you would swap the values. You could even destructure them instead of storing the previous value in a temporary variable: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Destructuring_assignment

Also, by moving the for loop into a different function, you only have to do this numArray[i] = MIN; if necessary.

Naming:

Your variables are lowercase, lower_case, camelCase, and UPPERCASE. I suggest standardizing on one. For one thing, MIN looks like a constant to me (from a PHP background), but it is being changed every time in the for loop. Also, what about improving the names: totalNum is unclear to me. Without looking beyond the naming is that an accumulator that stores the sum of all the items? temp could be previousItem.

Other Items:

• It's typically recommended to declare variables at the top of the function. j is declared numerous times.
• You initialize MIN at 0, but then reset it inside of the for loop. Why not just declare MIN and not set a value.

A completely different approach:

Finally, I wondered about using Javascript's Array.forEach(). The following is a completely different perspective, but I thought I would throw it out there for you to consider:

selectionSort();
function selectionSort(){
 var originalArray = [4, 34, 2, 5, 12],
 sortedArray = [],
 lowerSortedIndex;
 
 
 originalArray.forEach(function(item) {
 // Arrow function with implied return:
 lowerSortedIndex = sortedArray.findIndex((nextItem) => nextItem >= item);
 
 if (lowerSortedIndex >= 0) {
 sortedArray.splice(lowerSortedIndex, 0, item);
 } else {
 sortedArray[sortedArray.length] = item;
 }
 });
 console.log(sortedArray);
}

The arrow function could be re-written as:
function(nextItem) { return nextItem >= item; })

I wrote it here if you want to play with it: http://codepen.io/bassplayer7/pen/oBxmbW

• James Snell: If you're going to go the route of creating an array min function then javascript provides one
• bassplayer7: @JamesSnell one difference, though, is that findIndex() in my example is finding the item that is the next one bigger than the current one. So it's not the largest or smallest. Perhaps it would be even simpler to go based off of the min/max, although without trying it I'm not sure.
• James Snell: you're probably right, there could be (repeated values for example could prove tricky - I've not looked in detail.)
• bassplayer7: Yes, in my CodePen example, I've thrown negative numbers and duplicates at it and it still spits it out correctly.