1

I was trying to solve a problem where I had to sort so I used the standard library std::sort function but I get a wrong output in the 2nd test case:

#include <bits/stdc++.h> 

using namespace std;

int main()
{
    int t;
    cin>>t;
    while(t--)
    {
        int n,i;
        cin>>n;
        int arr[n-1];

        for(i=1;i<=n-1;i++)
            cin>>arr[i];

        int size=sizeof(arr)/sizeof(arr[1]);
        sort(arr,arr+size); 

        for(i=1;i<=n-1;i++)
            cout<<arr[i]<<" ";

        cout<<endl;         
    }
    return 0;
}

I/P:

2
5
1 2 5 4  
10
1 2 3 4 5 6 7 8 10

Expected O/P:

1 2 4 5
1 2 3 4 5 6 7 8 10

Actual O/P:

1 2 4 5
2 3 4 5 6 7 8 2013562 10
Blaze
  • 16,736
  • 2
  • 25
  • 44
Ashish Roy
  • 33
  • 4
  • 7
    `int arr[n-1];` is not allowed in Standard C++ and even if it was: you write out of bounds of the array (the last element has index `n-2`), and you sort a garbage element since you never set a value for `arr[0]` – M.M Jul 08 '19 at 07:23
  • 10
    First please read [Why should I not #include ?](https://stackoverflow.com/questions/31816095/why-should-i-not-include-bits-stdc-h) Secondly note that C++ doesn't really have [variable-length arrays](https://en.wikipedia.org/wiki/Variable-length_array), it's a non-portable extension in a few compilers. Use [`std::vector`](https://en.cppreference.com/w/cpp/container/vector) instead. Lastly, don't forget that array indexes are zero-based. So an array of `n - 1` elements will have indexes from `0` to `n - 1 - 1` (inclusive). – Some programmer dude Jul 08 '19 at 07:24
  • 3
    And if you really want to learn to program C++, get [a couple of good books](https://stackoverflow.com/questions/388242/the-definitive-c-book-guide-and-list/388282#388282) instead of spending time on online judge or competition sites. – Some programmer dude Jul 08 '19 at 07:25
  • 2
    Here is a more "idiomatic" version of your code: https://wandbox.org/permlink/3rXhYUyILiZDN3Zm This is not an answer to your question (see @Blaze's answer which is correct), but rather a way to improve your code. – Holt Jul 08 '19 at 08:32

1 Answers1

10

First of all, int arr[n-1]; is a variable length array. That's not actually part of C++, even though some compilers will tolerate it nevertheless. In most cases you can just use std::vector<int>(n-1); instead.

But look at this loop:

for(i=1;i<=n-1;i++)
    cin>>arr[i];

You're starting at 1 and got all the way up to n-1, however your array goes from arr[0] to arr[n-2]. So you got undefined behavior because you're writing one past the size of the array, and you also don't write to the first position (leading to more undefined behavior when you try to sort with this uninitialized value still present).

Instead, the loop should be for(i=0;i<n-1;i++) The same applies to where you're printing it. You can then sort the vector this way:

sort(arr.begin(), arr.end());

Also note that by doing n-1 you're always reading in and handling one less value that the user inputs, I'm not sure if that's your intention. If you want that, you could also just decrease n by one after reading it in instead of writing n-1 in multiple places.

Blaze
  • 16,736
  • 2
  • 25
  • 44
  • 1
    Looking at the input and expected output, it looks like `n - 1` is correct, but that's kind of weird... – Holt Jul 08 '19 at 08:31