Best Before puzzle

I'm new to C++ and decided to have a go at the spotify challenges on their website, http://www.spotify.com/uk/jobs/tech/best-before/

I have now finished but I get the feeling my code is just terrible, I'm guessing it would be very hard for someone else to read and I feel like there are much better ways to code this. If someone could kindly have a look and help me improve my code I would be very thankful.

Here is my code for the best before puzzle:

#include 
#include 
#include 
#include 
using namespace std;

int Year;
int Month;
int Day;

bool loop=true;
int date[4];
bool DateFound=false;
stringstream ss;
string input;
string in;

void Check_date();
void Check_date()
{
    if(DateFound==false)
    {
        //Check if valid Year
        if (date[1]0 && date[2]>0 && date[3]>0)
        {   //check months & days are valid
            if(date[2]==1 || date[2]==3 || date[2]==5 || date[2]==7 || date[2]==8 || date[2]==10 || date[2]==12){if(date[3]28)DateFound=false;
            }
        }
        if(DateFound==true)
        {
            Year=date[1]; Month=date[2]; Day=date[3];
            if(Year<1000)Year=Year+2000;
        }
    }
}

void SwitchDate(){int temp; temp=date[2]; date[2]=date[3]; date[3]=temp;   Check_date();};
void ShiftDate(int places)              
{
    if(places==1)
    {   
        int temp; temp=date[3]; date[3]=date[2]; date[2]=temp; temp=date[1]; date[1]=date[2]; date[2]=temp;  Check_date();
    }   
    if(places==2)
    {       
        int temp; temp=date[1]; date[1]=date[2]; date[2]=temp; temp=date[2]; date[2]=date[3]; date[3]=temp; Check_date();
    }       
};

And Main

```
int main ()
{
//Main Loop
while(loop==true)
{
cout >input;
cout> date[y];
ss.clear();
}

//order small medium large
for (int x=3, temp; x!=0; x--)
{
if (date[x] date[3] ))
{
temp=date[3];
date[3]=da

Initial comments on just reading:

Don't do this

using namespace std;

Yes every crappy book on C++ has this line.

Once you get past 5 lines programs it becomes a nuisance (technical term polluting the global namespace). So get in to the habbit of not using it. There are a couple alternatives (read other C++ posts on this forum) personally I prefix anything in standard with std:: (i.e. std::cout)

Global Variables are not a good idea.

int Year;
int Month;
int Day;

bool loop=true;
int date[4];
bool DateFound=false;
stringstream ss;
string input;
string in;

They bind your code to global state which makes modify your code relly hard and writting unit tests and validation code next to imposable. The best practice is a function/methods should not use any external objects. It either is in the scope of the function/method or is passed as a parameter.

Get into the habit of breaking really long lines into small chunks to make it more readable. Also you nested if and its sub-statement all on the same line are a real no-no. It is very hard to read.

if(date[2]==1 || date[2]==3 || date[2]==5 || date[2]==7 || date[2]==8 || date[2]==10 || date[2]==12){if(date[3] comment this way                                                                                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

Better would have been:

if(date[2]==1 || date[2]==3 || date[2]==5 || date[2]==7 || date[2]==8 || date[2]==10 || date[2]==12)
        {
            if(date[3]<=31)
            {    DateFound=true;
            }
        }

Even better would have been:

if(date[2]==1 || date[2]==3 || date[2]==5 || date[2]==7 ||
           date[2]==8 || date[2]==10 || date[2]==12)
        {
            if(date[3]<=31)
            {    DateFound=true;
            }
        }

Also notice:

if(date[3]<=31)
            {    DateFound=true;
            }

Can be written as:

DateFound = date[3]<=31;

There are lots of standard function that can make life easier:

Examples:

Here you are doing a swap

void SwitchDate(){int temp; temp=date[2]; date[2]=date[3]; date[3]=temp;   Check_date();};

Try:

void SwitchDate(){std::swap(date[2],date[3]); Check_date();};

Next you are implementing a simple bubble sort:

//order small medium large
    for (int x=3, temp; x!=0; x--)
    {
        if (date[x]  date[3] ))
        {
            temp=date[3];
            date[3]=date[2];
            date[2]=temp;
        }
    }

Try:

std::sort(&date[0], &data[3]);

Next you are implementing a sort of rotation threw the different combinations (using switchDate() and ShiftDate()). This functionality can be achieved using std::next_permutation.

Look here for a list of standard functions:

• index

• table of content

• std::swap

• std::sort

• std::next_permutation

With boolean expressions there is no point in testing a boolean against true/false. The point in making the variable boolean is so that it can be used directly and it should be named appropriately so that it's meaning is clear.

This is fine:

if(DateFound==false)

But a lot of people find the more succinct style more readable:

if(!DateFound)

As with the if statement above:

while(loop==true)

More concisely written as:

while(loop)

Not sure why you are writing the endl after reading the input.

cout >input;
    cout<<endl;

All it does is flush the output buffer. Which has already been flushed (because cin/cout are tied by magic that makes sure the user can read the question before answering).

You want to read three numbers divided by a slash?

for (int x=0, y=1; y> date[y];
        ss.clear();
    }

The stream operators can do much of the work for you. Basically it looks like this:

std::cin >> date[0] >> std::noskipws >> sep[0] >> date[1] >> sep[1] >> date[2];

Notice that your inner while loop has been replaced by a single read:

while (input[x] !='/' && x !=input.length()) ss> number;

But to take into account error detection and correction a bit of extra work is needed. So this is how I would do it. (Note its the comments that make it so big).

```
// Read one line of user input
// I always read a line of user input into a string then parse the string
// This makes error detection and recovery easier as the input stream is never
// in a bad state or needs to be reset. Also it swallows the new line character
// that can cause problems if you are not being careful.
std::string line;
std::getline(std::cin, line);

// Now that I have a lone of input I want parse it.
std::stringstream linestream(line);
int date[3]; // I want three numbers
char sep[2]; // Seporated by '/'

// Try and read the values you want.
linestream >> date[0] >> std::noskipws >> sep[0] >> date[1] >> sep[1] >> date[2];
if (!linestream || sep[0] != '/' || sep[1] != '/')
{
// linestream will be bad if reading any

StackExchange Code Review Q#9883, answer score: 18