Why is my code not sorting the data on the 2nd index at the array?

Question

I am trying to sort the members of the array in increasing order but i noticed that my code won't just sort the item at the 2nd index (i.e) element "51" however if i put in {31,41,59,26,41,58} in as my input it works just fine.

#include<bits/stdc++.h>
using namespace std;
int main()
{
    int A[6]={51,2,4,6,1,3};
    int temp;
    int var;
int j;
int length = 6;
    for(j=2;j<length;j++)
    {
        int key;
        key = A[j];
        int i;
        i=j-1;
        while(i>=0 && A[i]>key)
        {
            temp = A[i+1];
            A[i+1]=A[i];
            A[i]=temp;
            i=i-1;
        }
    A[i+1]=key;

    }
for(var =0;var<6;var++)
{
    cout<<A[var]<<endl;
}
return 0;
}

@IlyaBursov IN the SORTED array the 2nd element seems to have problem as output. — Dixon, Aug 11 '17 at 17:00
what sorting algorithm you're trying to implement? I really cannot recognize — Iłya Bursov, Aug 11 '17 at 17:03
then you have at least two problems: `j` should start from `1`, `A[i]` should be compared with `A[i-1]`, not with `key`, just compare your implementation with canonical one: https://en.wikipedia.org/wiki/Insertion_sort — Iłya Bursov, Aug 11 '17 at 17:07
Off topic: Read [Why should I not #include ?](https://stackoverflow.com/questions/31816095/why-should-i-not-include-bits-stdc-h) and [Why is “using namespace std” considered bad practice?](https://stackoverflow.com/questions/1452721/why-is-using-namespace-std-considered-bad-practice). I'm not too much of a hardcase on the second, but when you put the two of these together you include pretty much the entire standard library and you pull it all into the global namespace. That's a lot of identifiers and a lot of potential for unexpected nastiness. — user4581301, Aug 11 '17 at 17:54

Hariom Singh · Answer 1 · 2017-08-11T17:58:53.607

3

Just change (You got confused with indexing )

0 index is the first element

for(j=2;j<length;j++)

to

for(j=1;j<length;j++)

Apart from that changed

#include<bits/stdc++.h> ( It's GCC internal header used for precompiled header support)

to

#include<iostream>

After changing that

#include<iostream>
using namespace std;
int main()
{
    int A[6]={51,2,4,6,1,3};
    int temp;
    int var;
    int j;
    int length = 6;
    for(j=1;j<length;j++)
    {
        int key;
        key = A[j];
        int i;
        i=j-1;
        while(i>=0 && A[i]>key)
        {
            temp = A[i+1];
            A[i+1]=A[i];
            A[i]=temp;
            i=i-1;
        }
        A[i+1]=key;

    }


    for(var =0;var<6;var++)
    {
        cout<<A[var]<<endl;
    }
    return 0;
}

Output

1
2
3
4
6
51
Program ended with exit code: 0

edited Aug 11 '17 at 17:58

answered Aug 11 '17 at 17:05

Hariom Singh

3,512
6
28
52

`A[i]>key` is wrong comparison, test with different arrays and you will see it – Iłya Bursov Aug 11 '17 at 17:08
@ Ilya Bursov its upto the user what logic he wants to use ..answer is just to help in understand indexing ..there are several ways to sort :) – Hariom Singh Aug 11 '17 at 17:11
You should compare with A[i-1] instead of key – Iłya Bursov Aug 11 '17 at 17:19
@ Ilya Bursov 4 2 1 3 6 51 Program ended with exit code: 0 – Hariom Singh Aug 11 '17 at 17:23
@ user4581301 I didn't knew as I never used – Hariom Singh Aug 11 '17 at 17:58

R Sahu · Answer 2 · 2017-08-11T17:26:19.900

You can print the contents of the array at the end of each iteration in the for loop to diagnose where the error is in your code.

#include <iostream>

using namespace std;

template <int N>
void print(int (&arr)[N])
{
   for(int var =0; var<N; var++)
   {
      cout << arr[var] << " ";
   }
   cout << endl;
}

int main()
{
   int A[6]={51,2,4,6,1,3};
   int temp;
   int j;
   int length = 6;
   for(j=2;j<length;j++)
   {
      int key;
      key = A[j];
      int i;
      i=j-1;
      while(i>=0 && A[i]>key)
      {
         temp = A[i+1];
         A[i+1]=A[i];
         A[i]=temp;
         i=i-1;
      }
      A[i+1]=key;

      cout << endl << "Final result:" << endl;
      print(A);
   }

   print(A);

   return 0;
}

Output:

51 2 4 6 1 3
51 2 4 6 1 3
1 51 2 4 6 3
1 51 2 3 4 6

Final result:
1 51 2 3 4 6

I saw the following errors in your logic.

Your for loop needs to start with j=1, not j=2. Remember that the index 1 is used to access the second element of the array, not the first element of the array.

   for(j=1;j<length;j++)
   {
      ...
   }

The conditional of the while is not correct. It breaks the loop too early. The iteration of the elements and the swapping of the elements need to be separated.

      while(i>=0)
      {
         if ( A[i]>A[i+1] )
         {
            temp = A[i+1];
            A[i+1]=A[i];
            A[i]=temp;
         }
         i=i-1;
      }

I did not follow the reason behind the existence of the variable key. I think it can be removed. YMMV.

Here's an updated version of the program and its output.

#include <iostream>

using namespace std;

template <int N>
void print(int (&arr)[N])
{
   for(int var =0; var<N; var++)
   {
      cout << arr[var] << " ";
   }
   cout << endl;
}

int main()
{
   int A[6]={51,2,4,6,1,3};
   int temp;
   int j;
   int length = 6;
   for(j=1;j<length;j++)
   {
      int i;
      i=j-1;
      while(i>=0)
      {
         if ( A[i]>A[i+1] )
         {
            temp = A[i+1];
            A[i+1]=A[i];
            A[i]=temp;
         }
         i=i-1;
      }

      print(A);
   }

   cout << endl << "Final result:" << endl;
   print(A);

   return 0;
}

Output:

2 51 4 6 1 3
2 4 51 6 1 3
2 4 6 51 1 3
1 2 4 6 51 3
1 2 3 4 6 51

Final result:
1 2 3 4 6 51

Why is my code not sorting the data on the 2nd index at the array?

2 Answers2