Bubble sort examination

Question

I solved bubble sort in an exam, but the examiner wasn't satisfied with the solution. Please tell me why my solution is unsatisfactory.

function bubbleSort(myArray){
 var temp = undefined,isSwap = false;
 for(var x = 0; x < myArray.length; x++){
 if(((x+1) <= (myArray.length - 1)) && myArray[x] > myArray[x+1]){
 temp = myArray[x];
 myArray[x] = myArray[x+1];
 myArray[x+1] = temp;
 isSwap = true; 
 }else if(isSwap === true && x === (myArray.length - 1)){
 x = -1;
 isSwap = false;
 }else{
 continue;
 }
 }
 return myArray;
}

Just to see the performance I added a print statement i.e. console.log in the start of the loop like below

for(var x = 0; x < myArray.length; x++){
 console.log(myArray);

This is what I am able to see for the specific input:

console.log(bubbleSort([5,1,6,2,0]));

Output:

rahul@rahul:~/myPractise/Algo$ node sorting.js 
[ 5, 1, 6, 2, 0 ]
[ 1, 5, 6, 2, 0 ]
[ 1, 5, 6, 2, 0 ]
[ 1, 5, 2, 6, 0 ]
[ 1, 5, 2, 0, 6 ]
[ 1, 5, 2, 0, 6 ]
[ 1, 5, 2, 0, 6 ]
[ 1, 2, 5, 0, 6 ]
[ 1, 2, 0, 5, 6 ]
[ 1, 2, 0, 5, 6 ]
[ 1, 2, 0, 5, 6 ]
[ 1, 2, 0, 5, 6 ]
[ 1, 0, 2, 5, 6 ]
[ 1, 0, 2, 5, 6 ]
[ 1, 0, 2, 5, 6 ]
[ 1, 0, 2, 5, 6 ]
[ 0, 1, 2, 5, 6 ]
[ 0, 1, 2, 5, 6 ]
[ 0, 1, 2, 5, 6 ]
[ 0, 1, 2, 5, 6 ]
[ 0, 1, 2, 5, 6 ]
[ 0, 1, 2, 5, 6 ]
[ 0, 1, 2, 5, 6 ]
[ 0, 1, 2, 5, 6 ]
[ 0, 1, 2, 5, 6 ]
[ 0, 1, 2, 5, 6 ]

• Zeta: "Please tell me why my solution in unsatisfactory". You should ask your examiner why they weren't satisfied.
• Heslacher: Please state in the title of your question only what the purpose of your code is.

Answer 1

As @Zeta said, only your examiner can tell you why he wasn't satisfied. But here are a couple of suggestions anyway:

• Variable names are important and myArray doesn't mean much. You can try inputArray for example.
• Changing the index of a for loop is usually not a good idea, because it makes the code a bit more difficult to understand (not by a lot, but why make it harder?).

This is another go at the same algorithm, which I think is more readable:

function bubbleSort(inputArray)
{
 var index;
 var done = false;
 var temp;
 while ( !done )
 {
 done = true;
 for(var index = 0; index < inputArray.length; index++)
 {
 if ( inputArray[index] > inputArray[index+1] )
 {
 temp = inputArray[ index ];
 inputArray[ index ] = inputArray[ index+1 ];
 inputArray[ index+1 ] = temp;
 done = false;
 }
 }
 }
 return inputArray;
} 

Answer 2

function bubbleSort(myArray){
 var temp = undefined,isSwap = false;
 for(var x = 0; x < myArray.length; x++){
 if(((x+1) <= (myArray.length - 1)) && myArray[x] > myArray[x+1]){
 temp = myArray[x];
 myArray[x] = myArray[x+1];
 myArray[x+1] = temp;
 isSwap = true; 
 }else if(isSwap === true && x === (myArray.length - 1)){
 x = -1;
 isSwap = false;
 }else{
 continue;
 }
 }
 return myArray;
} 

Lets go from top to bottom.

• declaring multiple variables on the same line reduces the readability of your code. Readability is a key aspect in programming. If a reader of your code can't grasp at first glance what your code is doing then you have done something wrong.

• you should declare your variables as near to their usage as possible. This means e.g you should declare the temp variable inside the first if block.

• you are using 2 times myArray.length -1 so it would be better to extract this to a variable.

• The swapping of the array elements should be done in a separate method so its intent is more clear and your bubbleSort() method will be shorter.

• the continue in the else part is superflous because the loop wouldn't do anything different than continue.

Answer 3

Your code following some clean up that @Heslacher explained.

function bubbleSort(inputArray) {
 var n = inputArray.length;
 var hasSwapped = false;
 for (var i = 0; i < n; i++) {
 if (i < n - 1 && inputArray[i] > inputArray[i + 1]) {
 var temp = inputArray[i];
 inputArray[i] = inputArray[i + 1];
 inputArray[i + 1] = temp;
 hasSwapped = true;
 } else if (hasSwapped && i === n - 1) {
 i = -1;
 hasSwapped = false;
 }
 }
 return inputArray;
}

Attention for

1. Spacing
2. Unnecessary code
3. Variable names
   1. Prefer i for counters
   2. Avoid myXyz for anything
   3. isSwap is not exact what you want to inform
4. Complex boolean expressions
5. Avoid parenthesis on boolean expression ===, == have lower precedence than other operators.

Operators precedence in Javascript.