Sorting Odd And Even Elements From 2-D ARRAY

[akshatmahajan3112]

I AM trying to sort odd and even elements from 2-D array and show the output.
I think my logic is correct,there is some problem With the output?
Anyways Here Goes The CODE

// Write A Program To Sort Out Odd And Even Elements From A User Input 2-D Array
#include<iostream>
#include<stdlib.h>
using namespace std;
void sortoddeven (int a[][10], int r , int c)
{
    int i,j,k=0,l=0, b[10], d[10];
    for (i = 0 ; i < r ; i++)
    {
        for (j = 0 ; j < c ; j++)
            cin>>a[i][j];
    }

    for (i = 0 ; i < r ; i++)
    {
        for (j = 0 ; j < c ; j++)
            if (a[i][j] % 2 == 0)
            a[i][j] = b[k++];
        else
            a[i][j] = d[j++];
    }
    cout<<"\nEven Elements From The 2-D ARRAY";
    for (i = 0 ; i < r / 2; i++)
    {
        cout<<"\t"<<b[i];

    }
    cout<<"\nOdd Elements From The 2-D ARRAY:";
    for (i = 0 ; i < r / 2 ; i++)
    {
        cout<<"\t"<<d[i];
    }
}
int main()
{
    system("cls");
    int m,n,out,arr[15][10];
    cout<<"\nEnter The Number Of Rows";
    cin>>m;
    cout<<"\nEnter The Number Of Columns";
    cin>>n;
    sortoddeven(arr,m,n);
    return 0;
}

[icy1]

You should rewrite using hard-coded values because:
1. The focus of this post is extracting odd and even from 2D array, not user input
2. Your extraction function should be doing one thing and one thing only -- extracting odds and evens. It shouldn't also be asking user input
3. Easier to compare input and expected output with other people
4. Eventually, once you think the function is working with hard-coded values, if you still want user input, use vectors. Only vectors for dynamic array size -- your current code wouldn't support user input of 20, 20 for example.

[icy1]

Also, eventually you'll want to take input from an fstream, as it's much more convenient if lots of input is expected. For the purposes of the project, it doesn't matter how many rows the file is -- we can read each number easily, as long as whitespace separates the tokens. Here I've imitated an ifstream by using the very similar istringstream:

#include <iostream>
#include <vector>
#include <sstream>
#include <algorithm>

using namespace std;

void Show(const vector<int>& v)
{
    for (auto& n : v)
        cout << n << " ";
    cout << endl;
}

int main() 
{
    const char* data_text = R"LITERAL(59 100 40 72 49 56 93 92 50 89
62 99 86 96 70 95 93 78 81 39
)LITERAL";

    // Imitate file stream
    istringstream iss(data_text);
    
    vector<int> odds;
    vector<int> evens;
    int n;
    while (iss >> n)
    {
        if (n&1)
            odds.push_back(n);
        else
            evens.push_back(n);
    }
    sort(odds.begin(), odds.end());
    sort(evens.begin(), evens.end());
    cout << "Odds: ";
    Show(odds);
    cout << "Evens: ";
    Show(evens);
    
    return 0;
}

Can test/run at https://repl.it/repls/ScornfulOtherRegression

output:

Odds: 39 49 59 81 89 93 93 95 99 
Evens: 40 50 56 62 70 72 78 86 92 96 100

[Handy Andy]

Hello akshatmahajan3112,

I offer some suggestions that might help now and in the future.

Starting in main:

Line 36. It is best not to use "system" anything as it could leave your program vulnerable to hackers and not all people are able to use "system" as it is more of a Windows command.

Line 37. it is best not to use a single letter for a variable name as it is hard to understand and easy to miss. "m" and "n" this may mean something to you, but not the rest of us. Something like "numRows" and "numCols" makes more sense. "out" is never used. "arr" is usefum, but "numArr" or something like that is better. The "15" and "10" i OK, but it would work better by defining constexpr std::size_t MAXROWS{ 15 }; and the same for "MAXCOLS". Put this above main under the using namespace std; that is best not to use. As a global variable both the functions and main have access to these variables.

Since your array has a set size the prompts needs to reflect this limit something like: Enter The Number Of Rows 2 to 15 and the same concept for the columns. This way someone is less likely to enter a number that is beyond the size of the array. You should also follow each input with a check that the number entered is within the proper range.

Line 42. When I first look at the function name I think it is a variable. When I name things common variables start with a lower case letter. If put two or more words together I use the camel case method, e.g., "numRoes" and "numCols" this tends to be the easiest. Some people will use the '_', underscore, to in place of a space, but it is best to avoid this as it could become a conflict with something defined in a header file.

So instead of callint the 'SortOddEven" function you should first call something like "GetUserInput(numArr)". As ick1 mentioned this function should only do one thing and that is get the input from the user. In the end you are passing a pointer to the array and this should work fine, but I like to pass by reference. One of the biggest reasons is that I can see the whole array in the function not just the first element. To pass by reference it would look like this: GetUserInput(int (&numArr)[MAXROWS][MAXCOLS]). When passing by reference you have to tell the compiler the size of each dimension even a 1D array. Using the "MAXROWS" makes this very easy.
When you have your input then you can call the "sort" function to separate the numbers.

Now for the "sortoddeven" function.

Line 5. in the function definition using "r" and "c" is OK, but call it "rows" and "cols" it is much more descriptive of what it is.

Line 7. "i" and "j" should not be defined here, but in the for loop and I would call them "row" and "col". "k" is OK, but would be nice if it had a better name. "l" this is one letter to stay away from because in some type faces the lower case "L" looks to much like the number one and very easy to confuse.

For the two arrays what is "b" and "d"? It is better to call one "even" and the other "odd". It makes the code easier to understand otherwise it takes some work to figure out where the even numbers are going.

The first for loop should be in its own function.

The nested for loop would look better like this:

for (int row = 0; row < rows; row++)
{
	for (int col = 0; col < cols; col++)
		if (a[row][col] % 2 == 0)
			a[row][col] = even[k++];
		else
			a[row][col] = d[l++];
}

The only problem I see here for now if the last line of the inner for loop where it says "j++". I believe you meant to use the lower case "L" here not "j". The other problem is that both the even and odd array is very likely to go past the end boundary of the array and cause a run time error.

Thinking about the above and the next two for loops the size of the even and odd arrays are to small. The 2D array could have a maximum of 150 elements, so even if it is 50/50 that would give you at a minimum 75 elements in the array. I would not use 75 as the size of the array, I would consider giving the even and odd arrays a size of 150 and keep a count of how many elements are actually used. This is a place where using a vector has more flexibility if you can use a vector. At least with a vector you do not have to define a size when created you just add to it.

In the last two for loops in the middle section using "r / 2" is not the way to do this. Considering the the maximum value of "r" would be 15. "15 / 2" is 7.5 in integer division this would be 7 as the .5 would be dropped, so in an array of 10 where all 10 elements are use you would only be printing the first 7. Not what you want. What you do have is a variable that has counted how many times something was stored in the even and odd array and the is "k" and "l". So, instead of "r / 2" use "k" and "l" which has the number of elements used for each array.

At this point all of this is the right idea, but untested. It will take me a little while to get this program loaded up and changed around before I can test it.

Hope that helps,

Andy

[Handy Andy]

Hello akshatmahajan3112,

I have been working on your program not to give everything away to early this is the opening output of what I had in mind:

Enter The Number Of Rows 2 to 15: 20

error message

Enter The Number Of Rows 2 to 15: 15

Enter The Number Of Columns 2 to 10: 15

error message

Enter The Number Of Columns 2 to 10: 10

As you see if you enter the Rows incorrectly a while loop will trap the answer and loop until you enter a number in the correct range. This is done for each input.

Once you get the input working the way you want I used this to skip the input function and go directly to the sort function:

int numArr[MAXROWS][MAXCOLS]
{
	{ 49, 44, 60, 76, 19, 98, 76, 43, 93, 43 },
	{ 64, 26, 39, 94, 19, 28, 66, 3, 24, 57 },
	{ 32, 66, 18, 53, 9, 35, 90, 95, 90, 41 },
	{ 85, 56, 48, 84, 80, 97, 47, 41, 25, 46 },
	{ 64, 63, 32, 48, 43, 81, 41, 9, 53, 93 },
	{ 96, 99, 1, 79, 90, 63, 61, 94, 82, 43 },
	{ 48, 66, 73, 89, 94, 15, 77, 22, 63, 69 },
	{ 20, 77, 43, 56, 75, 92, 6, 34, 72, 45 },
	{ 95, 87, 29, 89, 66, 40, 26, 10, 79, 91 },
	{ 70, 85, 57, 18, 4, 6, 94, 82, 84, 42 },
	{ 7, 45, 57, 38, 60, 99, 89, 66, 54, 98 },
	{ 67, 33, 32, 52, 35, 47, 31, 70, 95, 78 },
	{ 5, 82, 77, 41, 83, 44, 21, 66, 90, 48 },
	{ 28, 71, 75, 10, 81, 28, 77, 91, 94, 31 },
	{ 100, 60, 22, 99, 8, 53, 23, 73, 84, 92 }
};

This is 150 random numbers and produces this output:

Enter The Number Of Rows 2 to 15: 15

Enter The Number Of Columns 2 to 10: 10

Even Elements From The 2-D ARRAY
44      60      76      98      76      64      26      94      28      66      24      32      66      18      90
90      56      48      84      80      46      64      32      48      96      90      94      82      48      66
94      22      20      56      92      6       34      72      66      40      26      10      70      18      4
6       94      82      84      42      38      60      66      54      98      32      52      70      78      82
44      66      90      48      28      10      28      94      100     60      22      8       84      92

Odd Elements From The 2-D ARRAY:
49      19      43      93      43      39      19      3       57      53      9       35      95      41      85
97      47      41      25      63      43      81      41      9       53      93      99      1       79      63
61      43      73      89      15      77      63      69      77      43      75      45      95      87      29
89      79      91      85      57      7       45      57      99      89      67      33      35      47      31
95      5       77      41      83      21      71      75      81      77      91      31      99      53      23
73

That is 74 even numbers and 76 odd numbers for this array.

Hope that helps,

Andy

[akshatmahajan3112]

Thank You Everyone Especially Andy and Icy1,I finally got a bit of understanding of vectors and having less problems working on such programs since. Thank You Everyone for you time.