HiveBrain v1.2.0
Get Started
← Back to all entries
patterncppMinorCanonical

"Best before" puzzle

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
beforepuzzlebest

Problem

I'm new To C++ and decided to have a go at the Spotify challenges on their website.

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.

Their "Best Before" puzzle asks: Given a possibly ambiguous date written in "A/B/C" format, where A,B,C are integers between 0 and 2999, interpret them as M/D/Y or D/M/Y or whatever common date format. Pick the interpretation that yields the earliest possible legal date between Jan 1, 2000 and Dec 31, 2999 (inclusive) in "YYYY-MM-DD" format. If the string cannot be interpreted as a valid date, state so.

Here is my code for the best before puzzle:

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

int Year;
int Month;
int Day;

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

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>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]=date[2];
                    date[2]=temp;
                }

    }

    Check_date();
    SwitchDate();
    ShiftDate(1);
    SwitchDate();
    ShiftDate(2);
    SwitchDate();

    //PRINT
    if(DateFound==true)
    {
    cout >in;
    cout << endl;
    if(in=="y" || in=="Y"){loop=true;}
    if(in=="n" || in=="N"){loop=false;}
    }//End of Loop
}

Solution

-
You should avoid using global variables. If you import this code into a future project it might cause problems -- name conflicts etc.

If you must do this, put them in an anonymous namespace so that they're only in scope for this file:

namespace
{
    int my;
    int globals;
    int defined;
    int here;
}


-
The declaration of Check_date() is not necessary here:

void Check_date();
void Check_date()
{


-
This could perhaps written better:

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


There is little point using x in the for loop as you don't have an exit condition for it. Also, I would prefer a more meaningful name.

Instead, perhaps:

int inputPos = 0;
for(int dateIndex = 1; dateIndex > date[dateIndex];
        ss.clear();
    }
    ++inputPos;
}


-
More generally you've written this in a very C-like style -- I would have defined a type (class) for Date, which would have month and day members. Then the logic for whether month lengths are valid can be encapsulated properly.

You could also encapsulate the read-from-stream code using an iostreams extractor.

Code Snippets

namespace
{
    int my;
    int globals;
    int defined;
    int here;
}
void Check_date();
void Check_date()
{
for (int x=0, y=1; y<=3; y++, x++)
{
    while (input[x] !='/' && x !=input.length()) ss<<input[x++];
    ss>> date[y];
    ss.clear();
}
int inputPos = 0;
for(int dateIndex = 1; dateIndex <= 3; ++dateIndex)
{
    while(input[inputPos] != '/' && inputPos != input.length()) 
    {
        ss >> date[dateIndex];
        ss.clear();
    }
    ++inputPos;
}

Context

StackExchange Code Review Q#9915, answer score: 4

Revisions (0)

No revisions yet.