current community

your communities

more stack exchange communities

company blog
Stack Exchange Inbox Reputation and Badges
tour help
Stack Overflow is a community of 4.7 million programmers, just like you, helping each other.

Join them; it only takes a minute:

Sign up
Join the Stack Overflow community to:
  1. Ask programming questions
  2. Answer and help your peers
  3. Get recognized for your expertise
up vote 1 down vote favorite

Please can you tell me what is wrong to this implementation of bubble sort algorithm in JavaScript?

for (var i=1; i<records.length; i++){
        for (var j=records.length; j<1; j--){
            if (parseInt(records[i-1]) < parseInt(records[i])){
                var temp = records[i-1];
                records[i-1] = records[i]
                records[i] = temp;
            }   
        }    
    }
share|improve this question
2  
This might work better if you tell us what the problem is with it, and then we might be able to tell your how to fix it. – Matt♦ Sep 21 '11 at 15:33
    
@Mansuro Well i think so isn't it? – kostas trichas Sep 21 '11 at 15:37
up vote 2 down vote

Here's an algorithm that works:

function swap(items, firstIndex, secondIndex){
    var temp = items[firstIndex];
    items[firstIndex] = items[secondIndex];
    items[secondIndex] = temp;
}

function bubbleSort(items){

    var len = items.length,
        i, j, stop;

    for (i=0; i < len; i++){
        for (j=0, stop=len-i; j < stop; j++){
            if (items[j] > items[j+1]){
                swap(items, j, j+1);
            }
        }
    }

    return items;
}

Check reference here!

share|improve this answer
up vote 1 down vote 
for (var j=records.length; j<1; j--){

Shouldn't that be

for (var j=records.length; j>1; j--){
share|improve this answer
    
i think it should be (records.length - 1) as the index start from 0 – Ujjwal Manandhar Sep 21 '11 at 15:34
1  
But j isn't used as an index into an array; rather it is used as a counter – davbryn Sep 21 '11 at 15:36
    
is that really a bubble sort ? how will that work ?? – Ujjwal Manandhar Sep 21 '11 at 15:41
    
It won't bubble sort at the moment - it will just keep swapping the i and (i - 1)th element. I was just pointing out the obvious problem with his code (the reason the inner loop wasn't being entered). – davbryn Sep 21 '11 at 15:46
    
yeah i was pointing that too :) – Ujjwal Manandhar Sep 21 '11 at 15:47
up vote 1 down vote

you should use j instead of i in the second loop, and don't forget to change the j<1 to j>1

share|improve this answer
    
Well thank you, i think i use J instead of I – kostas trichas Sep 21 '11 at 15:42
1  
if (parseInt(records[i-1]) < parseInt(records[i])){ var temp = records[i-1]; records[i-1] = records[i] records[i] = temp; that's i there – Mansuro Sep 21 '11 at 15:44
    
Yes you are right, but then i get: TypeError: records[j] is undefined – kostas trichas Sep 21 '11 at 15:46
    
that's because you are trying to access records[records.length], which doesn't exist in this array, if you want to start the for loop from the end, you have to start with records.length-1 for (var j=records.length-1; j>0; j--) – Mansuro Sep 21 '11 at 15:59
up vote 0 down vote

the second for loop is coded wrong it should be

for (var i=0; i<records.length; i++){
    for (var j=0; j<records.length; j++){
        if (parseInt(records[i]) > parseInt(records[j])){
            var temp = records[i];
            records[i] = records[j];
            records[j] = temp;
        }   
    }    
}
share|improve this answer
    
It is better to nest your for loop inside a while loop and establish a predicate for looping. The code above will continue looping and looping, till it ends... even once the list elements have already been placed in order. – shmuli Sep 25 '15 at 20:21
up vote 0 down vote

I believe that in a bubble sort, once the i loop has completed an iteration, then the i'th element is now in its correct position. That means that you should write the j loop as

for (var j = i + 1; j < records.length; j++)

Otherwise your bubble sort will be (even more) inefficient.

share|improve this answer
up vote 0 down vote

My solution:

  function bubbleSort(A){
  var swapped,
      len = arr.length;

  if(len === 1) return;

  do {
    swapped = false;
    for(var i=1;i<len;i++) {
      if(A[i-1] > A[i]) {   
        var b = A[i];
        A[i] = A[i-1];
        A[i-1] = b;
        swapped = true;
      }   
    }

  }
  while(swapped)  
}

var arr = [1, 6, 9, 5, 3, 4, 2, 12, 4567, 5, 34];
bubbleSort(arr);
document.write(arr);
share|improve this answer

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.