I didn't know why my array is displaying wrong numbers. Just want to know why is that so?

Question

Didn't know why my odd array is displaying some large number. I want to print only the odd numbers from the array in a sorted manner.
Like if the array is 1 4 6 8 0 9
Print only 1 9
selectionSort() is just the function that sorts the array.

int main()
{
    int T, n, p, size,sum=0,si=0;
    cin >> T;
    for (int i = 0; i < T; i++)
    {
        cin >> n;
        int a[n];
        int odd[n];
        for (int j = 0; j < n; j++)
        {
            cin >> a[j];
        }
        for (int j = 0; j < n; j++)
        {
            p = 0;
            if (a[j] % 2 != 0){
                odd[p++] = a[j];
                si++;
            }
        }
        selectionSort(odd, si);

What is wrong here in for loop?

    for (int k = 0; k < si; k++)
    {
        cout << odd[k] << endl;
        // sum += odd[j];
    }

        // cout << sum << endl;
        sum = 0;
        si=0;
    }
    return 0;
}

Output is :
1
4
1 5 7 9
9
16
4200276
6422112

Expecting 
1
4
1 5 7 9
1
5
7
9

Questions seeking debugging help should generally include a [mre] of the problem (complete code, including all `#include` directives). — Andreas Wenzel, Nov 14 '21 at 19:25
`int a[n];` is not standard C++. You therefore need to give more details about the toolset you are using. — Bathsheba, Nov 14 '21 at 19:27
Note that `int a[n];` is not standard C++. Variable length arrays are only possible with compiler extensions. See also [Why aren't variable-length arrays part of the C++ standard?](https://stackoverflow.com/q/1887097/11082165) — Brian61354270, Nov 14 '21 at 19:27
Simplify. It the problem in building the array of odd numbers, sorting it, or displaying it? Remove the call to `selectionSort` and see if you get the right numbers. Instead of looping through the input, initialize the array of add numbers with some values; see if you get the right result. Try hard-coding some input instead of reading from the console; see if you can isolate the problem that way. — Pete Becker, Nov 14 '21 at 19:29
If you want to learn C++, first write your code with `vector` and use `.at(i)` for indexing. Then if it's correct you can think whether it's worth to optimize for speed. But please none of this `int x[n]` stuff. It's really C coding, not idiomatically C++. — Victor Eijkhout, Nov 14 '21 at 20:23

Drew Dormann · Answer 1 · 2021-11-14T19:59:52.370

0

This loop:

    for (int j = 0; j < n; j++)
    {
        p = 0;
        if (a[j] % 2 != 0){
            odd[p++] = a[j];
            si++;
        }
    }

Only ever assigns to odd[0]. You conditionally increment p, but then always set it to 0 again.

Since your code expects si to match the count of valid entries in odd, your code can be simplified to:

    for (int j = 0; j < n; j++)
    {
        if (a[j] % 2 != 0){
            odd[si++] = a[j];
        }
    }

edited Nov 14 '21 at 19:59

answered Nov 14 '21 at 19:44

Drew Dormann

59,987
13
123
180

Yep same issue I've stated, but there is another one, if the condition is false, odd[p++] is never initalized and the value will be undefined. You can initalize the odd array like this odd[n] = {0} but this is specific to the version of C++ you are using because this isn't standard – IwillbeBetter Nov 14 '21 at 19:48
@IwillbeBetter in the case you describe, `si` will also be `0`, and therefore the array won't be printed. If I'm not mistaken. – Drew Dormann Nov 14 '21 at 19:53

score 0 · Accepted Answer · answered Nov 14 '21 at 22:41

0

p=0 is not necessary in the for loop. odd[p++] = a[j] equation always mean odd[0]=a[j]. Put p=0 out of the for loop.

answered Nov 14 '21 at 22:41

emtsdmr

29
2

score -4 · Answer 3 · answered Nov 14 '21 at 19:29

-4

You are declaring an array with a variable, which shouln't be allowed but I don't know which compiler you are using so I won't go too deep onto that.

You've never initialized the odd array, the values inside of it are undefined

answered Nov 14 '21 at 19:29

IwillbeBetter

83
1
4

2

The value in the odd array don't have to be initialized if the code writes values into the array and only uses values that have already been written. – Pete Becker Nov 14 '21 at 19:30
The values on odd are only written if (a[j] % 2 != 0), plus he inserting values inside of odd using p, which is set to 0 each iteration, thus only odd[0] is being written, and the other values are undefined – IwillbeBetter Nov 14 '21 at 19:42
If `odd[0]` has been set to a value, writing out the value of `odd[0]` is okay. If you see something else going on there you need to say what that is. – Pete Becker Nov 14 '21 at 20:14

I didn't know why my array is displaying wrong numbers. Just want to know why is that so?

3 Answers3