js Bubble sort not processing all elements correctly

Question

I pass in elements array [5,4,3]. The bubble sort manages to push 5 the end, this is fine, but when it loops through a third time to locate the second to last element, the loop breaks and the incorrect ordered array is returned... not sure why, cheers guys

this.bubbleSort = function (elements) {

        //Loop through to the second to last index. By the time we get to the last index, its already //been compared with what’s in front of it
        var hasHadChange;
        for (var x = 0; x < elements.length - 1; x++) {
            hasHadChange = false;

            //Loop through to the second to last index.
            for (var y = 0; y < elements.length - 1; y++) {

                //Check the current item(x) in the array plus the item next to current item (x+1), if its larger
                if (elements[y] > elements[y + 1]) {

                    //Acknowledge there has been a change
                    hasHadChange = true;

                    //Swap the items around
                    var temp = elements[y];
                    elements[y] = elements[y + 1];
                    elements[y + 1] = temp;
                    //This continues until the largest value has bubbled to the right
                }
            }
        }
        return elements;
    }

you are using same variable name (x) in both loops leading to a strange effect. change one of them. and you are not using hasHadChange figure out its importance and use it. — dsharew
– dsharew, Commented Dec 8, 2014 at 12:53
Thanks for the quick replies, ive separated the vars in to x & Y, but still the loop breaks, an no matter what order i push my values they get returned the same — Rich J
– Rich J, Commented Dec 8, 2014 at 14:14
so update the above code with the new one so we can see if you have another bug. — dsharew
– dsharew, Commented Dec 8, 2014 at 14:30
listen to @vjdhama and accept his answer if it works for you. — dsharew
– dsharew, Commented Dec 8, 2014 at 15:34

Guffa · Accepted Answer · 2014-12-08 12:50:59Z

1

You need to use separate variables for the inner and the outer loop. When you exit the inner loop, x will be equal to the length, so the outer loop will end also.

answered Dec 8, 2014 at 12:50

Guffa

703k112 gold badges760 silver badges1k bronze badges

Sign up to request clarification or add additional context in comments.

Comments

vjdhama · Accepted Answer · 2014-12-08 19:14:54Z

You should use separate variables in inner and outer loops. Using y in inner loop instead will give you the correct answer.

var bubbleSort = function (elements) {

    //Loop through to the second to last index. By the time we get to the last index, its already //been compared with what’s in front of it
    var hasHadChange;
    for (var x = 0; x < elements.length - 1; x++) {
        hasHadChange = false;

        //Loop through to the second to last index.
        for (y = 0; y < elements.length - 1; y++) {

            //Check the current item(x) in the array plus the item next to current item (x+1), if its larger
            if (elements[y] > elements[y + 1]) {

                //Acknowledge there has been a change
                hasHadChange = true;

                //Swap the items around
                var temp = elements[y];
                elements[y] = elements[y + 1];
                elements[y + 1] = temp;
                //This continues until the largest value has bubbled to the right
            }
        }
    }
    return elements;
}

The reason behind this is variable hoisting.

When a variable is declared, it breaks into two parts. One part moves to top of it's scope, other stays at it's position. For Example,

function foo() {
    if (false) {
        var x = 1;
    }
    var y = 1;
}

Will look like :

function foo() {
    var x = undefined;  //Declaration of x is hoisted
    var y = undefined;   //Declaration of y is hoisted
    if (false) {
        x = 1; //Assignment still occurs where we intended
    }
    y = 1; //Assignment still occurs where we intended
}

This is what happened in the code. Using same variable in both loops makes them overwrite each other values. Hence the result.

From ECMAScript standard 5.1 :

A variable statement declares variables that are created as defined in 10.5. Variables are initialised to undefined when created. A variable with an Initialiser is assigned the value of its AssignmentExpression when the VariableStatement is executed, not when the variable is created.

See this MDN doc for more details. Look for topic var hoisting.

Update

Using let which has block level scope, you can have variable x in both loops.

var bubbleSort = function (elements) {

    //Loop through to the second to last index. By the time we get to the last index, its already //been compared with what’s in front of it
    var hasHadChange;
    for (var x = 0; x < elements.length - 1; x++) {
        hasHadChange = false;

        //Loop through to the second to last index.
        for (let x = 0; x < elements.length - 1; x++) {

            //Check the current item(x) in the array plus the item next to current item (x+1), if its larger
            if (elements[x] > elements[x + 1]) {

                //Acknowledge there has been a change
                hasHadChange = true;

                //Swap the items around
                var temp = elements[x];
                elements[x] = elements[x + 1];
                elements[x + 1] = temp;
                //This continues until the largest value has bubbled to the right
            }
        }
    }
    return elements;
}

Although the variable is naturally hoisted, that isn't really relevant. Are you thinking of variable scope?
Let was added in Javascript 1.7 and is in ES6 draft. Might not have been implemented in setting(chrome) you are working on. But I think mozilla has implemented it. I tried using it on mozilla console and it worked. I guess you shouldn't use that for now.

tomasb · Accepted Answer · 2014-12-08 12:52:33Z

0

You are using the same control variable x for both for-cycles, should use different like x and y for example.

answered Dec 8, 2014 at 12:52

tomasb

1,6732 gold badges23 silver badges30 bronze badges

Collectives™ on Stack Overflow

js Bubble sort not processing all elements correctly

3 Answers 3

Comments

2 Comments

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

3 Answers 3

Comments

2 Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Related