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

Enter year, date, and month in any order, I'll organize it

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

Problem

I would like some advice on how I could make it less repetitive and use more functions and classes.

```
#include
#include

using namespace std;

class date
{
public:
date() = default;
date(istream &is) { is >> year >> month >> day; }

bool organizeInfo();
bool organize();
ostream &outputInfo(std::ostream &os) { os << year << " " << month << " " << day; return os; }

private:

bool checkValid();
bool checkSwitch(unsigned &item1, unsigned &item2, unsigned &item3);
void switchItems(unsigned &item1, unsigned &item2) { unsigned tempItem1 = item1; item1 = item2; item2 = tempItem1; }

unsigned year = 0;
unsigned month = 0;
unsigned day = 0;
};

inline bool date::checkValid()
{
if (month <= 12 && day <= 31)
return 1;
return 0;
}

inline bool date::checkSwitch(unsigned &item1, unsigned &item2, unsigned &item3)
{
switchItems(item1, item2);
if (checkValid())
return 1;
else
{
switchItems(item1, item2);
switchItems(item2, item3);
if (checkValid())
return 1;
else
{
switchItems(item2, item3);
switchItems(item1, item3);
if (checkValid())
return 1;
else
{
return 0;
}
}
}
}

inline bool date::organize()
{
if (checkValid())
return 1;
else
{
if (checkSwitch(day, month, year))
return 1;
else if (checkSwitch(year, month, day))
return 1;
else if (checkSwitch(day, year, month))
return 1;
return 0;
}
}

bool date::organizeInfo()
{
if (checkValid())
return 1;
else
{
if (organize())
return 1;
else
return 0;
}
return 0;
}

int main()
{
cout << "Enter year, month, date and ill try and organize it" << endl;
date dawg(cin);
if(dawg.organizeInfo())
dawg.outputInfo(cout)

Solution

Besides the obvious points already mentioned in the other answers

Don't populate member variables in a constructor from streamed input

Passing std::istream to populate members in the constructor looks weird:

date(istream &is) { is >> year >> month >> day; }


The canonical way is to provide an input operator:

class date {
     friend std::istream& operator>>(std::istream& is, date& d) {
          is >> d.year >> d.month >> d.day;
          return is; 
     }
     // ....
};


This allows to check the stream state for input errors after using it.

Don't promise things to users of your API, you cannot ever guarantee

As going for the logic you're trying to implement:

You are aware that it's not really possible to distinguish day, month and year inputs just by the bare numbers?

Some test cases in day, month, year order:

  • 1 3 2



  • 5 11 23



  • ...



Well, month must be below or equal 12 and days below or equal 31, but what about days up to 12 or years input up to 12/31?

You actually have to require a specific date format for user input

Users won't be happy with wrong guesses from your code, and what you're promising isn't possible at all.

This is a major flaw in your code, and renders any other discussion merely useless.

Code Snippets

date(istream &is) { is >> year >> month >> day; }
class date {
     friend std::istream& operator>>(std::istream& is, date& d) {
          is >> d.year >> d.month >> d.day;
          return is; 
     }
     // ....
};

Context

StackExchange Code Review Q#114468, answer score: 16

Revisions (0)

No revisions yet.