Design
You use three values to represent a date Year/Month/Day. This makes nearly all operations on the class complicated. The only simple operation is printing.
Since you have designed your interface around this structure it may be enough.
But I find the interface very lacking in what I would want to do (add arbitrary periods to the date, compare dates, etc). I think you will find most date classes will convert this into a single value and perform the operations on that value. Then most other operations will becomes simple and printing becomes complex. A single complex operation that is not used often (in comparison to function call count of other functions) would seems a better design.
But since you have not specified a clear use case for the date design sure it may be enough.
Error Handling
Come on this is obviously wrong:
if ((day <= 0) || (day >= 32)) {
cerr << "Day is limited between 1 and 31, you entered: " << day << endl;
day = month = year = 0;
return;
}
if ((month <= 0) || (month >= 13)) {
cerr << "Month is limited between 1 and 12, you entered: " << month << endl;
day = month = year = 0;
return;
}
if ((year <= 1899) || (year >= 2018)) {
cerr << "year is limited between 1900 and 2017, you entered: " << year << endl;
day = month = year = 0;
return;
}
Days and Month cannot be 0. Month cannot be 13 and days cannot be 32 (unless you are living on Pandora). Might be nice if these values were actually specified via a config file? That does make leap years hard to solve though. Also years are wrong based on the messages.
Your code contradicts your error messages. This is absolutely unacceptable. As a maintainer which do I believe the code or the English. Which do I correct? Where is the link to the documentation that I need to understand the code so it can be fixed?
You don't even handle leap years.
bool isLeapYear(int year) {
if (year < -46) // 46 BC
return false;
}
if (year < 1582) // Julian Calander
return year / 4 == 0;
}
if ((year >= 1582 and year <= 1923) && checkLocalIsUsingJulian(year)) // Gregorian Calendar not fully standard until 1923
return year / 4 == 0;
}
// Gregorian Calendar
// You may just want to assume a Gregorian calendar (most people do).
return (year / 400 == 0) || ((year / 4 == 0) && (year / 100 != 0));
}
Reading Input
The use of strtok() is not a good choice for code as it modifies the input parameters. scanf() is slightly better (and I could use that). But since this is C++ code I think it would be more obvious to use operator>> to read values from a stream.
void convert_date(char *date1){
char sep1;
char sep2;
char errorCheck;
std::stringstream data1stream(data);
if ( (data1stream >> d >> sep1 >> m >> sep2 >> y) // Read Data
&& (sep1 == '/' && sep2 == '/') // Correct seporators.
&& !(data1stream >> errorCheck)) // No junk at end of input.
{
// correctly read and formatted input
}
else
{
// Error
}
Code Review
There is a reason for these warnings:
#pragma warning(disable:4996)
You should not disable them. You should fix this warning. The warning is there because you are using outdated functions. Update your code to use the more modern versions.
Don't do this:
using namespace std;
Read this: Why is “using namespace std” considered bad practice?; best answer is here.
Constructor:
Date() {
day = month = year = 0;
}
This is a bad default. Especially since it is an illegal date. Why not default to today?
Initializer List
Prefer to use the initializer list (yes I know it will have no difference on this code). But when you start using classes it will have a difference. You want your code to look consistent no matter what the type is so using the initializer list will always give you the same code and always by the optimal way to do it.
Date(int day, int month, int year) {
this->day = day;
this->month = month;
this->year = year;
}
// Prefer this:
Date(int day, int month, int year)
: day(day)
, month(month)
, year(year)
{}
Never use this->
Using this-> will hide shadowing errors.
The only reason to use this-> is to deference a variable that is more locally scoped than a member variable. But the problem here is that if you forget then there is no compiler warning to diagnose it (your code just has a bug).
A better approach is to never have shadowed variables. Then you will never need to use this-> to differentiate between the two and it will always be correct. You can also get the compiler to error when there is a shadowed variable.
Learn to use std::string
You have C-Strings splashed around your code. These are very inefficient to use (in comparison to std::string as you have to compute their length all the time). But additionally they are error prone because you need to manually set the length of the underlying input buffer being used.
char date1[11];
cin.getline(date1, 11);
That works. But what if I change the size of the buffer? Now I have to search the application for all instances of 11 to make sure all the places it is used are updated correctly.
Now we have to find the actual length:
unsigned length = strlen(date1);
It would have been easier to write:
std::string date1;
std::getline(std::cin, date1);
Prefer range-based for:
for (unsigned i = 0; i < length; ++i) {
In modern C++ we prefer the range-based for:
for (auto const& c: date1) {
Prefer '\n' over std::endl
cerr << "Error: Alphabet character found: " << date1[i] << endl;
The difference between the two is a buffer flush to the stream. Manual flushing is discouraged (as it is nearly always wrong). The system will flush the streams for you at the most optimum time.
If you look at most speed question on SO you will find that inappropriate flushing is one of the main reasons for C++ code being slower than C. Remove the inappropriate flushes and the C++ code will get to the same speed as C.
Don't use strtok()
It modifies the input data.
DRY your code.
DRY: Don't repeat your self.
This code is don in two places. It would be better if it was made into a separate method and called from both locations. That way when you want to fix the bug you only have to fix the bug in one location.
if ((day <= 0) || (day >= 32)) {
cerr << "Day is limited between 1 and 31, you entered: " << day << endl;
day = month = year = 0;
return;
}
if ((month <= 0) || (month >= 13)) {
cerr << "Month is limited between 1 and 12, you entered: " << month << endl;
day = month = year = 0;
return;
}
if ((year <= 1899) || (year >= 2018)) {
cerr << "year is limited between 1900 and 2017, you entered: " << year << endl;
day = month = year = 0;
return;
}
Learn to use const methods:
int get_day(void) {
return day;
}
int get_month(void) {
return month;
}
int get_year(void) {
return year;
}
Methods that do not modify the internal state of the object should be marked const; this allows them to be called on const objects. Which then helps when you pass objects by const reference to other methods.
Don't wait to do this later - adding const later has a horrible tendency to cascade changes through the code. Do it now, as you design the class.
Define your own print operator.
t.print_date();
Why can't I go:
std::cout << t;
All you need to do is define the output operator:
std::ostream& operator<<(std::ostream& str, Date const& date)
{
date.print_date(str); // You may want to change `print_date()`
// to accept a stream as a parameter so
// it can be printed on any stream not
// just std::cout
return str;
}
