CODEWITHSUNDEEP
javascriptbubble-sort
Mazri_02
Mazri_02

Reputation: 25

Why does My Bubble sort code always show me repeated numbers?

Recenly I tried to test my own Bubble Sort code using JavaScript but the problem is everytime I run it the output still show me the same result. It repeatedly print the last index in the array. Can anyone help me out, I'm still new at this.

By the way here's the code.

var num = [31,23,55,2,13,90];
var bilnum = num.length,i ,j;
var temp = num[0];

for(i = 0;i < bilnum; i++){
    for(j = 0 ;j < bilnum - i; j++){
        if(num[j] < num[j+1]){
            num[j] = temp;
            num[j] = num[j+1];
            temp = num[j];
            
        }
    }
}

document.write(num)

Upvotes: 1

Views: 83

Answers (4)

Daniel
Daniel

Reputation: 15443

Your problems begin in the second for loop:

for(j = 0 ;j < bilnum - i; j++){

Why?

The minus i is what's going to account for the fact that after you do a full iteration through your array the first time, the right most element will be in the correct location, but you forgot the - 1 like so:

for(j = 0 ;j < bilnum - i - 1; j++){

Your second bug is here:

if(num[j] < num[j+1]){

it should be:

if(num[j] > num[j+1]){

Why?

This is the part where we are now doing the comparison and swapping logic. Remember, you are comparing two number pairs each go around.

So you are going to look at element j and the element right next to it which would be j + 1.

You are going to see if element j is greater and if it is, you are going to swap the two elements.

So that's what's going to compare the left hand side to the right hand side of each pair.

If the left hand side is greater, then you need to do a swap of these two elements.

As such your second mistake is here:

num[j] = temp;

Instead it should be:

temp = num[j];

And so your final mistake is the last line:

temp = num[j];

You are successfully taking the right hand side and throwing it over to the left, but you are not taking the left hand side and throwing it over to the right with the above, instead you want this:

 num[j + 1] = temp;

I think what can help you in the future is because of the fact that temp is poorly named, rename it to leftHand.

That makes it more obvious that this is a reference to the element on the left hand side of each pair.

Upvotes: 0

tarkh
tarkh

Reputation: 2549

Another approach to sort arrays manually, just for fun

// Manual sort function
const sort = ([...arr], type = 'asc', res = []) => {
  // Loop while array has values
  while(arr.length > 0) {
    // Define max and index variables
    let max = '', index = 0;
    // Loop existing values and find max value and
    // it's index.
    for(let i = 0; i < arr.length; i++) if(arr[i] > max) max = arr[i], index = i;
    // Cut max value from array and push it to the results
    res.push(arr.splice(index, 1)[0]);
  }
  // If you need ascending sort - revers results manually
  if(type === 'asc') res = ((a) => [...a].map(a.pop, a))(res);
  // Return result
  return res;
}

//
// Set and test
//

// Set test array of numbers
const num = [31,23,55,2,13,90];

// Test manual sort function
console.log(...sort(num, 'asc'));
console.log(...sort(num, 'desc'));

// Set test array of strings
const str = ['alpha', 'zetta', 'betta', 'xray', 'gamma', 'comma'];

// Test manual sort function
console.log(...sort(str, 'asc'));
console.log(...sort(str, 'desc'));

Upvotes: 0

Bhojendra Rauniyar
Bhojendra Rauniyar

Reputation: 85575

It's because, you are assigning num[j] = temp; and again for the same. So, it's going to show you last value from the array.

It should be:

var num = [31,23,55,2,13,90];
    var bilnum = num.length,i ,j;
    var temp = num[0];
    
    for(i = 0;i < bilnum; i++){
        for(j = 0 ;j < bilnum - i; j++){
            if(num[j] < num[j+1]){
              temp = num[j];            
              num[j] = num[j+1]; // first item
              num[j+1] = temp; // second item
            }
        }
    }
    console.log(num)

Upvotes: 1

David Buzatu
David Buzatu

Reputation: 644

Your problem is here:

for(i = 0;i < bilnum; i++){
  for(j = 0 ;j < bilnum - i; j++){
      if(num[j] < num[j+1]){
          num[j] = temp;
          num[j] = num[j+1];
          temp = num[j];
      }
  }
}

When you switch, who is temp at first? It is your first value (temp = num[0] in the beginning). It then takes the value of the first swap and so on, messing up your array and outputting the unexpected result.

When switching two elements, the order is like this:

temp = num[j] -> hold current value

num[j] = num[j + 1] -> swap with the next value

num[j + 1] = temp -> put the value from before in the new position

Also, your if condition will sort your array in descending order. Use: if (arr[j] > arr[j + 1]) to sort in increasing order

Your final code should look like this:

var num = [31,23,55,2,13,90];
var bilnum = num.length,i ,j;
var temp;

for(i = 0;i < bilnum; i++){
    for(j = 0 ;j < bilnum - i; j++){
        if(num[j] > num[j+1]){
            num[j] = temp;
            num[j] = num[j+1];
            temp = num[j];
        
        }
    }
 }

 document.write(num)

Upvotes: 0

Related Questions