Sorting an array with simple algorithm but loop becomes infinite

Question

I am trying to sort an array elements. I have made a very simple algorithm of mine. Here is it

<html>
<head>  

    <script>

    var numberList = [44, 58, 12, 53, 25, 63];  

    for(var i = 0; i<numberList.length; i++){
        var reset = i;
        for(var j = 1; j<numberList.length; j++){
            if(numberList[j]<numberList[i]){
                var k = numberList[i];
                var l = numberList[j];
                numberList[i] = l;
                numberList[j] = k;
                i++;
            } else{
                var k = numberList[i];
                var l = numberList[j];
                numberList[i] = k;
                numberList[j] = l;
                i++;
            }                   
        }   
        i = reset;      
    } 
    document.write(numberList); 

    </script>

</head>

<body>

</body>
</html>

But the problem is loop becomes infinite. Each time after execution of inner for loop, the value of variable i becomes 5. So I have introduced a variable named reset to restore the value of i. So that i is again set to it's primary value & then first for loop make an increment to it. But it becomes infinite. The same algorithm is working fine in another program. But here it's no. I appreciate your help.

why don't you just use Array.prototype.sort()? Why re-invent the wheel for something that's already built into javascript environment — charlietfl
– charlietfl, Commented Jul 16, 2015 at 4:39
there is not just 1 problem with algorithm. First of all its complexity is n^2 secondly you are incrementing at wrong place. — atinder
– atinder, Commented Jul 16, 2015 at 4:41
The else block is redundant, it's just putting the members in their current locations (i.e. it doesn't move them anywhere). — RobG
– RobG, Commented Jul 16, 2015 at 4:43
@charlietfl Most probably the poster is trying to sharpen his skill in JS I suppose. — Derek 朕會功夫
– Derek 朕會功夫, Commented Jul 16, 2015 at 4:44

RIYAJ KHAN · Accepted Answer · 2015-07-16 05:09:31Z

2

try this one

var numberList = [44, 58, 12, 53, 25, 63];  
    var intArrayLength = numberList.length;
    for(var i = 0; i <intArrayLength; i++){

        for(var j = i+1; j < intArrayLength; j++){

            if(numberList[i] > numberList[j] ){
                var swapNumber = numberList[j];
                numberList[j] = numberList[i];
                numberList[i] = swapNumber;
            }                  
        }   

    } 
    document.write(numberList);

EDIT :

inner for loop can be written in this way also without using temp variable.

for(var j = i+1; j < intArrayLength; j++){

            if(numberList[i] > numberList[j] ){
                numberList[j] += numberList[i];
                numberList[i] =  numberList[j]-numberList[i];
                numberList[j] =  numberList[j]-numberList[i];
            }                  
        }

edited Jul 16, 2015 at 5:09

answered Jul 16, 2015 at 5:02

RIYAJ KHAN

15.3k5 gold badges34 silver badges56 bronze badges

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

2 Comments

RobG Over a year ago

I don't see the point in avoiding the temporary variable, especially as the above method modifies the values when they're moved, so it might not restore the exact same values (and assumes that they are numbers). The temporary variable is only created once and its value changed on each iteration only if members are out of sequence. Simple assignment should be faster than a sequence of mathematic operations, even if they're simple addition and subtraction. Also, I think the algorithm is incorrect, try it with the smallest value at the largest index.

RIYAJ KHAN Over a year ago

@RobG I have given alternate option for swapping .I am not saying which one is faster or slow.And which one should use.

nick · Accepted Answer · 2015-07-16 05:08:56Z

2

When you increment i in scope of "j"-cycle, you do not check that i is less than array length. I.e. during execution of this cycle you refer to not existed element of array and add it to the array. So you need to change condition in the second for statement:

for(var i = 0; i<numberList.length; i++){
    var reset = i;
    for(var j = 1; j<numberList.length &&  i<numberList.length; j++){

So if we go step-by-step through you code, we can find the next:

i = 0, j = 1: [44, 58, 12, 53, 25, 63]
i = 1, j = 2: [44, 12, 58, 53, 25, 63]
i = 2, j = 3: [44, 12, 53, 58, 25, 63]
i = 3, j = 4: [44, 12, 53, 25, 58, 63]
i = 4, j = 5: [44, 12, 53, 25, 58, 63]

i = 1, j = 1: [44, 12, 53, 25, 58, 63]
i = 2, j = 2: [44, 12, 53, 25, 58, 63]
i = 3, j = 3: [44, 12, 53, 25, 58, 63]
i = 4, j = 4: [44, 12, 53, 25, 58, 63]
i = 5, j = 5: [44, 12, 53, 25, 58, 63]

i = 2, j = 1: [44, 53, 12, 25, 58, 63]
i = 3, j = 2: [44, 53, 25, 12, 58, 63]
i = 4, j = 3: [44, 53, 25, 58, 12, 63]
i = 5, j = 4: [44, 53, 25, 58, 63, 12]
i = 6, j = 5: // i == numberList.length

So that is a reason of yours infinite-loop

edited Jul 16, 2015 at 5:08

answered Jul 16, 2015 at 4:55

nick

6934 silver badges11 bronze badges

1 Comment

RobG Over a year ago

Actually answering the question? Now there's a novel concept…;-)

tning · Accepted Answer · 2015-07-16 05:09:47Z

2

for(var i = 0; i<numberList.length-1; i++){ 
   for(var j = i+1; j<numberList.length; j++){
        if(numberList[j]<numberList[i]){
            var k = numberList[i];
            var l = numberList[j];
            numberList[i] = l;
            numberList[j] = k;
        }                   
    }     
}

edited Jul 16, 2015 at 5:09

answered Jul 16, 2015 at 5:03

tning

1,26110 silver badges23 bronze badges

Comments

Andreas · Accepted Answer · 2015-07-16 04:56:52Z

0

Every time i is incremented, the value of reset is then set to the new i value.

When you have the line i = reset; it doesn't make any difference, because at the top you have already told it that reset = i. Try taking the reset variable out.

Also, the .length command will give back the number of values in the array. In this case length = 6. However, the final index of the array is actually number 5 because it starts at 0 (0, 1, 2, 3, 4, 5 = 6 indices). So when i is 5, it is still less than the length and so the code you have written will increment i to 6, which is out of bounds on the array.
To fix this, you could change the code to read...

for(var i = 0; i< (numberList.length-1); i++)

so that when i is 5, it wont be incremented any further because 5 is the last index value of the array.

I hope that helps. (and makes sense :) )

edited Jul 16, 2015 at 4:56

Andreas

21.9k7 gold badges52 silver badges58 bronze badges

answered Jul 16, 2015 at 4:50

Colin Adams

412 silver badges5 bronze badges

1 Comment

nick Over a year ago

for(var i = 0; i< (numberList.length-1); i++) won't help because this code refer to numberList[6], when initial value of i = 2, j = 5. I.e. as a result of the very last i++ i become equal to 6

Collectives™ on Stack Overflow

Sorting an array with simple algorithm but loop becomes infinite

4 Answers 4

2 Comments

1 Comment

Comments

1 Comment

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

4 Answers 4

2 Comments

1 Comment

Comments

1 Comment

Your Answer

Sign up or log in

Post as a guest

Related