Finding odd and even digits from an integer

I was given a task:


Given any integer - write a program, which:



-
finds all digits of the integer

-
writes odd and even digits from the integer

-
finds the biggest even and smallest odd digit

-
writes the biggest possible number that can be formed by rearranging the odd digits

-
writes the smallest possible number that can be formed by rearranging the even digits


I have written this code, but I am wondering if I can somehow get it written shorter than this:

#include 
#include 
using namespace std;
int main()
{
    long a,s=0,s1=0;
    int b,n=0,n1=0,k=0;
    int d[10],nl[10];
    int c=0;
    int i=0,j=0;
    for (i=0; i>a;
    i=0;
    while(a!=0) {
        b=a%10;
        a=a/10;
        if(b%2!=0) {
            d[i]=b;
            i++;
        }
        else {
            nl[k]=b;
            k++;
        }
    }
    n=i;
    n1=k;
    coutd[j])
            {
                c=d[i];
                d[i]=d[j];
                d[j]=c;
            }
        }
    }
    cout<<" Odd numbers ranged array"<<endl;
    for(i=0; i<n; i++)
        cout<<d[i]<<" ";
    cout<<endl;
    cout<<"Min odd number "<<d[n-1]<<endl;
    c=0;
    for(i=0; i<n1; i++)
    {
        for(j=0; j<n1; j++)
        {
            if(nl[i]<nl[j])
            {
                c=nl[i];
                nl[i]=nl[j];
                nl[j]=c;
            }
        }
    }
    cout<<" Even numbers ranged array"<<endl;
    for(i=0; i<n1; i++)
        cout<<nl[i]<<" ";
    cout<<endl;
    cout<<"Max even number "<<nl[k-1]<<endl;
    for(i=0; i<n; i++)
    {
        s=s*10+d[i];
    }
    for(i=0; i<n1; i++)
        s1=s1*10+nl[i];
    cout<<"Max odd digits' number "<<s<<endl;
    cout<<"Min odd digits' number "<<s1<<endl;
    return 0;
}

-
Try to avoid using using namespace std.

-
` is not utilized anywhere, so just remove it unless you end up adding associated code.

-
Avoid giving variables single-letter names (unless they're simple loop counters, which is okay to do). Although it may save some typing, it will instead make them appear meaningless as there is no context. Comments can be added, but they're not a good substitute for actual variable names. If a variable needs a comment, chances are it hasn't been given a good name.

-
To help increase readability and maintainability:

-
Have one variable per line. This also helps in case comments need to be added.

long a;
long s=0;
long s1=0;

-
Separate operators and operands with whitespace so that the two can be identified.

long a;
long s = 0;
long s1 = 0;

-
Separate code lines based on purpose, also so that readers can focus on certain parts.

cout<<" Odd numbers ranged array"<<endl;

for(i=0; i<n; i++)
    cout<<d[i]<<" ";

cout<<endl;

-
Keep variables as close in scope as possible instead of listed together. This could also help determine if a variable is no longer being used.

cout > a;

-
std::endl also flushes the stream, which is slower. If you want just a newline, use "\n":

cout << "Write a number: \n";

-
This:

c=nl[i];
nl[i]=nl[j];
nl[j]=c;

can be done more concisely with std::swap:

std::swap(n1[i], n1[j]);

-
Consider making this program more modular (using functions). Keeping everything in main() could make it hard for yourself and others to read and maintain the code. It is also not clear, just from looking at it, that this program is written to fulfill those five tasks. It could just be one task if the reader weren't otherwise aware of them.

This can be done any way you'd like, but the most likely solution could be to have one function for each of those tasks (while giving them appropriate names). They could then be called from main(), and you could also get the user input from main()` (this could, for instance, allow you to terminate the program right away if invalid input is given).

StackExchange Code Review Q#36505, answer score: 9