patterncppMinorCanonical
"Best before" puzzle
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:
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:
-
The declaration of
-
This could perhaps written better:
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:
-
More generally you've written this in a very C-like style -- I would have defined a type (class) for
You could also encapsulate the read-from-stream code using an iostreams extractor.
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.