I wanted to reverse my array. Why this code gives garbage value?

Question

    #include <iostream>


    using namespace std;

    int main(){

    int n;
    int a[n];
    cin>>n;
    for(int i=0;i<n;i++){
        cin>>a[i];
    }
    for(int i=n;i>=0;i--){
       cout<<a[i]<<" ";
   }
    }

Input:- 4 1 2 3 4 Output 4199008 4 3 2 1

Enable warnings on your compiler. `int a[n];` isn't supported by every compiler and should generate a warning, if not an error. You don't initialize `n` first. You actually want `std::vector`. Also, [do not use `using namespace std;`](https://stackoverflow.com/q/1452721/501250). You can reverse any container that supports bidirectional iteration using [`std::reverse()`](https://en.cppreference.com/w/cpp/algorithm/reverse). — cdhowie, May 22 '20 at 20:19
Also, your second loop indexes `a[n]`, which is UB. you need to start at `n-1`. — cigien, May 22 '20 at 20:20
Your code clearly does not attempt to "reverse the array". See std::reverse. After applying it, cout the resulting array, first to last. This simplifies and documents your coding intent, and avoids the unintended Undefined Behavior. — 2785528, May 23 '20 at 00:31

Vlad from Moscow · Accepted Answer · 2020-05-22T20:48:08.917

For starters the program has undefined behavior because the variable n is not initialized

int n;

So this declaration

int a[n];

is invalid. Moreover variable length arrays is not a standard C++ feature. Instead use the standard class template std::vector.

Also within this loop

for(int i=n;i>=0;i--) {
   cout<<a[i]<<" ";
}

you are trying to access of non-existent element with the index n.

Also you are not reversing an array. You are trying to output an array in the reverse order.

Pay attention to that there are standard algorithms std::reverse and std::reverse_copy declared in the header <algorithm>.

Here is an example how your program with using your approach could look

#include <iostream>
#include <vector>

int main() 
{
    size_t n = 0;

    std::cout << "Enter the size of an array ";

    std::cin >> n;

    std::vector<int> v( n );

    std::cout << "Enter " << n << " elements: ";

    for ( auto &item : v ) std::cin >> item;

    std::cout << "The array in the reverse order\n";

    for ( size_t i = v.size(); i != 0;  )
    {
        std::cout << v[--i] << ' ';
    }
    std::cout << '\n';

    return 0;
}

The program output might look like

Enter the size of an array 10
Enter 10 elements: 0 1 2 3 4 5 6 7 8 9
The array in the reverse order
9 8 7 6 5 4 3 2 1 0

If to use standard algorithms then your program can look the following way

#include <iostream>
#include <vector>
#include <iterator>
#include <algorithm>

int main() 
{
    size_t n = 0;

    std::cout << "Enter the size of an array ";

    std::cin >> n;

    std::vector<int> v( n );

    std::cout << "Enter " << n << " elements: ";

    std::copy_n( std::istream_iterator<int>( std::cin ), n, std::begin( v ) );

    std::cout << "The array in the reverse order\n";

    std::reverse_copy( std::begin( v ), std::end( v ), 
                       std::ostream_iterator<int>( std::cout, " ") );
    std::cout << '\n';

    return 0;
}

The program output might look the same way as shown above

Enter the size of an array 10
Enter 10 elements: 0 1 2 3 4 5 6 7 8 9
The array in the reverse order
9 8 7 6 5 4 3 2 1 0

Honorable mention: `rbegin()` and `rend()` which make `reverse_copy` completely unnecessary. — Ben Voigt, May 22 '20 at 21:40

score 1 · Answer 2 · answered May 22 '20 at 20:21

1

a[n] will return the element after the last one. When you iterate in reverse order, start with i=n-1.

answered May 22 '20 at 20:21

Andreas Haferburg

5,189
3
37
63

2

There's more that needs to be fixed, e.g. the `int a[n]` before `n` is initialized. Even initializing it would not work unless `n` is `const`. – cigien May 22 '20 at 20:22

score 0 · Answer 3 · answered May 22 '20 at 20:48

At the beginig of your program there is a mistake:

    int n; // You declare n with no value
    int a[n]; // You use is
    cin>>n; // After you used it you get your value-

Now i can suppose that that was just an error while copying it because you give inputs and outputs

Input:- 4 1 2 3 4 Output 4199008 4 3 2 1

So forgeting about that, you declare an array of size n. Remember that the elemnts of the array will go from 0 to n-1. Now look at your second for loop

    // the first element you acces is n and you stop at 1
    // but the array goes from n-1 to 0
    for(int i=n;i>=0;i--){
       cout<<a[i]<<" ";
    }

So you still get n values as an output but the first element that you access is outside of the array. Thats why you get a garbage value, that is a value that was left there.

A solution will be to change the for loop

    for(int i=n-1;i>=-1;i--){
       cout<<a[i]<<" ";
   }

Abhishek kumar · Answer 4 · 2020-05-22T20:59:32.063

0

While reversing the array start the loop from n-1 that is i=n-1 (n is the no of elements in array). And run the loop till i>=0. If you will start loop from n it will read illegal index which is out of range and will give you garbage value.

 for(int i=n-1; i>=0; i++){
      cout<<arr[i]<<" ";}

edited May 22 '20 at 20:59

answered May 22 '20 at 20:53

Abhishek kumar

139
7

I wanted to reverse my array. Why this code gives garbage value?

4 Answers4