# Bubble sort examination

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 ]

• "Please tell me why my solution in unsatisfactory". You should ask your examiner why they weren't satisfied.
– Zeta
Commented Jul 20, 2016 at 6:52
• Please state in the title of your question only what the purpose of your code is. Commented Jul 20, 2016 at 7:53

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;
}

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.

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.