From array of evens and odds make new array where evens first odds last

Question

Ok, so when I am printing the final array (arr2), the first element that has been copied from odd[] to arr2[] is a random number instead of it being an odd number that was inserted in arr1[]. Here's a picture as an example pic .

#include "stdafx.h"
#include <iostream>
#include <conio.h>
using namespace std;

void main()
{
    int arr[20], odd[20],arr2[20], i, j = 0, k = 0, no,temp,temp2,o=1;
    cout << "Size of Array: ";
    cin >> no;
    cout << "Enter any " << no << " elements in Array: ";
    for (i = 0; i<no;i++)
    {
        cin >> arr[i];
    }
    for (i = 0; i<no;i++)
    {
        if (arr[i] % 2 == 0)
        {
            arr2[j] = arr[i];
            j++;
            temp = j+1;
        }       
        else
        {
            odd[k] = arr[i];
            k++;
            temp2 = k;
        }
    }

    cout << endl;
    cout << "New array:" << endl;

    for (i = 1; i <= temp2; i++)
    {
        arr2[temp] = odd[o];
        temp++;
        o++;
    }

    for (i = 0;i < no;i++)
    {
        cout << arr2[i] <<endl;
    }


}

Welcome to Stack Overflow! Great that you gave a reproducible example! However, it is best practice to not link to images of console output but to include it as text. See http://idownvotedbecau.se/imageofcode — akraf, Dec 20 '17 at 12:15
@Ron why am i getting random output for the first odd number when i print arr2 — andrE, Dec 20 '17 at 12:17
It's probably because you have an uninitialized element in between. Did you debugging? — Scheff's Cat, Dec 20 '17 at 12:18
In addition to @manni66: it's `int main()` and it should `return` something (usually `0` in case of success and anything else otherwise). — Scheff's Cat, Dec 20 '17 at 12:20
Unless there is a requirement from my tutor/professor to use arrays i would use std::vector instead, specially if the number of elements is determined at runtime. That said, I would use a somewhat different algorithm for creating the second vector. I would keep track of the first and last empty slot to be able to put the even numbers in front and the odd ones at the end. — Johan, Dec 20 '17 at 12:20
@manni66 I know that for C but thought it's not true for C++. (Have no spec. at hand.) — Scheff's Cat, Dec 20 '17 at 12:22
@Scheff i changed it to int main() and return 0; but that doesn't seem to matter as my problem is when i print the array and the first odd number in arr2 is a random number — andrE, Dec 20 '17 at 12:26
@manni66 i am not allowed to use std::parition, i need a solution for this problem (still a beginner, don't even know how either) — andrE, Dec 20 '17 at 12:30
@Scheff C++ standard, section 9.6.3.2: "Flowing off the end of a constructor, a destructor, or a function with a cv `void` return type is equivalent to a return with no operand. Otherwise, flowing off the end of a function **other than `main` (6.6.1)** results in undefined behavior." — Algirdas Preidžius, Dec 20 '17 at 12:30
The hint about `int main` was correct but marked as OT (off-topic) as not relevant for your problem. Did you notice my earlier comment "It's probably because you have an uninitialized element in between. Did you debugging?"? An out-of-bound access could also be the reason e.g. in `for (i = 1; i <= temp2; i++)` (and watch the `o`). — Scheff's Cat, Dec 20 '17 at 12:31
@Scheff Yes it's a requirement to use arrays in this exercise — andrE, Dec 20 '17 at 12:32
Sorry, where did I mention not to use arrays? I recommended to use a debugger (beside of arrays). ;-) — Scheff's Cat, Dec 20 '17 at 12:33
@Scheff oh my bad i tagged you instead of Johan, i saw that my o was set at 1 instead of 0 but something is still wrong — andrE, Dec 20 '17 at 12:38
@andrE Did you try stepping through your code with a debugger, as already was suggested, to figure out what? — Algirdas Preidžius, Dec 20 '17 at 12:43
@Scheff in debugger it shows arr2[0] and arr2[1] are ok but arr2[2] is not getting any value and instead arr3[3] and arr2[4] are getting values — andrE, Dec 20 '17 at 12:53
@andrE So watch the index variables. And be aware that you might access elements which are not initialized or even behind end of array. The latter is [UB](https://stackoverflow.com/a/4105123/1505939) but does not mean that process crashs ASAP. Be tough - fight this through in debugger. (I've to do this all day...) Btw., my alternative solution (using only one array): [ideone.com](https://ideone.com/yj8ljK) — Scheff's Cat, Dec 20 '17 at 12:57
@Scheff fixed, temp=j+1 was the problem, should've been only temp=j, thank you so much for the help — andrE, Dec 20 '17 at 12:57
@Scheff: you are right: 3.6.1 [basic.start.main] says *If control reaches the end of main without encountering a return statement, the effect is that of executing `return 0;`* — Serge Ballesta, Dec 20 '17 at 12:58
@SergeBallesta OK. Actually manni66 was the one who knew - I was the one in doubt. Another one stated 6.6.1 but you said 3.6.1. Which one is correct? — Scheff's Cat, Dec 20 '17 at 13:00
@Scheff: Algirdas Preidžius used C++17 standard when I still use n4296 for C++14. Clauses have been renumbered between those versions. — Serge Ballesta, Dec 20 '17 at 13:03

score 0 · Accepted Answer · edited Dec 20 '17 at 13:12

0

I have not seen where your problem come from. However, there is a much easier way to do this using std::sort. Here is how I would do it:

#include <iostream>
#include <algorithm>
using namespace std;

bool sortingFunction(int left, int right)
{
    if (left % 2 == 0 && right % 2 == 0 || left % 2 != 0 && right % 2 != 0)
        return left<right;
    else if (left % 2 != 0 && right % 2 == 0)
        return false;
    else if (left % 2 == 0 && right % 2 != 0)
        return true;
}

int main()
{
    int no;
    cout << "Size of Array: ";
    cin >> no;
    int arr[no];
    cout << "Enter any " << no << " elements in Array: ";
    for (int i = 0; i<no;i++)
    {
        cin >> arr[i];
    }

    std::sort(arr,arr+no,sortingFunction);
    for (int i = 0; i< no; i++)
        cout<<arr[i]<<" ";
    cout<<endl;
    return 0;
}

edited Dec 20 '17 at 13:12

Scheff's Cat

19,528
6
28
56

answered Dec 20 '17 at 12:56

apalomer

1,895
14
36

I would use this as well but i am restricted from using it ( as stupid as it sounds ) and it needs to be stored into a new array. – andrE Dec 20 '17 at 13:04
1

My even shorter approach for `sortingFunction()`: `bool sortingFunction(int left, int right) { return left % 2 < right % 2; }` as requirement was "where evens first odds last". So, even against even need not to be sorted nor odd against odd. – Scheff's Cat Dec 20 '17 at 13:17
std::partition would be a better algorithm for that problem. – Dec 20 '17 at 13:29
@manni66 for OP's copy requirement [`std::partition_copy`](http://en.cppreference.com/w/cpp/algorithm/partition_copy) – Caleth Dec 20 '17 at 13:47
@Scheff you are completely right! I did not see it when programming my answer. – apalomer Dec 20 '17 at 15:07

score 0 · Answer 2 · answered Dec 20 '17 at 13:18

There are two errors

instead of temp = j+1, you should write temp = j.

As you increment j after adding value in arr2, j is already pointing at index after all even numbers.

Second is in third loop you use o as index but it is initialized with 1, it should be initialize with 0.

Your code should be like

void main()
{
  int arr[20], odd[20], arr2[20], i, j = 0, k = 0, no, temp, temp2, o = 0;
  cout << "Size of Array: ";
  cin >> no;
  cout << "Enter any " << no << " elements in Array: ";
  for (i = 0; i<no; i++)
  {
    cin >> arr[i];
  }
  for (i = 0; i<no; i++)
  {
    if (arr[i] % 2 == 0)
    {
      arr2[j] = arr[i];
      j++;
      temp = j;// +1;
    }
    else
    {
      odd[k] = arr[i];
      k++;
      temp2 = k;
    }
  }

  cout << endl;
  cout << "New array:" << endl;

  for (i = 1; i <= temp2; i++)
  {
    arr2[temp] = odd[o];
    temp++;
    o++;
  }

  for (i = 0; i < no; i++)
  {
    cout << arr2[i] << endl;
  }
  cin >> arr2[0];

}

From array of evens and odds make new array where evens first odds last

2 Answers2