Simple days between dates calculator

Question

I recently decided to try my hand at this problem of calculating the number of days between two given dates including leap years in the calculation. I chose to do it in C++ as it is the language I feel most comfortable with and was wondering about the quality of my code.

#include <iostream>
#include <vector>
#include <string>    

using namespace std;    

typedef struct Date{
    int day;
    int month;
    int year;    

    Date(int d, int m, int y):day(d), month(m), year(y)
    {
    }    

    bool operator== (const Date& rhs)
    {
        return day==rhs.day && month==rhs.month && year==rhs.year;
    }
}Date;    

vector<int> splitBy(string input, char delimiter)
{
    vector<int> output;
    int startDayIndex = 0;
    int endIndex = 0;
    int length = input.length();
    while(input.find(delimiter, startDayIndex)!=string::npos)
    {
        endIndex = input.find(delimiter, startDayIndex);
        output.push_back(stoi(input.substr(startDayIndex,endIndex)));
        startDayIndex = endIndex+1;    

    }    

    output.push_back(stoi(input.substr(startDayIndex,length)));
    return output;
}    

int NextMonth(int currentMonth)
{
    if(currentMonth == 12) return 1;
    return currentMonth+1;
}    

int NextYear(int month, int currentYear)
{    

    return month==1?currentYear+1:currentYear;
}    

bool isLeapYear(int year)
{
    if(year%4!=0) return false;
    else if(year%100!=0) return true;
    else if(year%400!=0) return false;
    else return true;
}    

int DaysInMonth(int month, int year)
{
    vector<int> days = {31, 28, 31, 30, 31, 30, 31, 31, 30, 31, 30, 31};
    if(isLeapYear(year)) days[1] = 29;
    return days[month-1];    

}    

bool errorCheck(Date start, Date end)
{    

    if(start.year>end.year) return true;
    else if(start.month>end.month && start.year==end.year) return true;
    else if(start.day>end.day && start.month==end.month && start.year==end.year) return true;
    return false;
}    

int start(string firstDate, string endDate)
{
    vector<int> initDate = splitBy(firstDate, '/');
    vector<int> finDate = splitBy(endDate, '/');
    Date startDay(initDate[0], initDate[1], initDate[2]);
    Date endDay(finDate[0], finDate[1], finDate[2]);
    if(errorCheck(startDay,endDay)) return -1;
    int count = 0;
    int days = DaysInMonth(startDay.month, startDay.year);    

    while((startDay==endDay)!=1)
    {
        while(startDay.day<days)
        {
            startDay.day = startDay.day + 1;
            count++;
            if((startDay==endDay)==1)
            {
                return count;
            }
        }
        count++;
        startDay.day = 1;
        startDay.month = NextMonth(startDay.month);
        startDay.year = NextYear(startDay.month,startDay.year);
        days = DaysInMonth(startDay.month, startDay.year);
    }
    return count;

}    


int main()
{
    cout << "Enter date in format DD/MM/YYYY\n";
    string startDayDate;
    string endDate;
    cout << "Enter first date\n";
    cin >> startDayDate;
    cout << "Enter second date\n";
    cin >> endDate;
    int result = start(startDayDate, endDate);
    result==-1?cout << "Invalid input\n":cout << "The number of days are: " << result << endl;

    return 0;

}

I know off the bat that using namespace std is an issue, are there any other issues with regards to readability, efficiency, etc.? Does the code I have written to accomplish this task qualify as "good" code?

Answer 1

I see a number of things that may help you improve your code.

Don't abuse using namespace std

Putting using namespace std at the top of every program is a bad habit that you'd do well to avoid.

Sanitize user input better

The date check could be a little more robust. In particular, verifying that the month is in the range of 1 to 12 is particularly important because that number is later used as an index into a vector. If I enter the nonsensical date 40/40/1940 the errorCheck routine currently accepts it as valid and the program later crashes. If I enter just 40/40, the program also crashes.

Simplify your code

Right now the code includes this line:

while((startDay==endDay)!=1)

It would be more straightforward to simply write this:

while(startDay != endDay)

This would only require the addition of an operator!= to the Date.

Don't complicate the code

The code currently contains this structure declaration:

typedef struct Date{
    // stuff
}Date;

First, the typedef is neither needed nor useful. This isn't C. Second, it would make sense to declare it as a class rather than as a struct.

class Date {
    // stuff
};

Use const where practical

The operator== member function of Date does not alter the underlying object and therefore should be declared const.

bool operator==(const Date& rhs) const;

Make your classes do more work!

The current Date class is a very lazy thing, leaving all of the work to separate functions. I'd make it do much more of the work. For instance, I reimplemented your algorithm but did most of the work in class operators. Here's an excerpt of that code:

int operator-(Date end, Date start) {
    bool swapped{false};
    if (end < start) {
        std::swap(start, end);
        swapped = true;
    }
    int diff = 0;
    for ( ; start != end; ++start) {
        ++diff;
    }
    return swapped ? -diff : diff;
}

int main() {
    Date start, end;

    std::cout << "Enter start date in format DD MM YYYY\n";
    std::cin >> start;
    std::cout << "Enter ending date in format DD MM YYYY\n";
    std::cin >> end;
    std::cout << end-start << "\n";
}

Consider the cost of object creation

The code currently contains this function:

int DaysInMonth(int month, int year)
{
    vector<int> days = {31, 28, 31, 30, 31, 30, 31, 31, 30, 31, 30, 31};
    if(isLeapYear(year)) days[1] = 29;
    return days[month-1];    
}    

This is computationally much more costly than it should be because it requires the creation and destruction of a std::vector on every invocation. Those numbers don't change, so that first line could instead look like this:

static constexpr int days[12]{31, 28, 31, 30, 31, 30, 31, 31, 30, 31, 30, 31};

In the case of a leap year, alter the returned value instead of the array itself.

Omit return 0

When a C or C++ program reaches the end of main the compiler will automatically generate code to return 0, so there is no need to put return 0; explicitly at the end of main.

Note: when I make this suggestion, it's almost invariably followed by one of two kinds of comments: "I didn't know that." or "That's bad advice!" My rationale is that it's safe and useful to rely on compiler behavior explicitly supported by the standard. For C, since C99; see ISO/IEC 9899:1999 section 5.1.2.2.3:

[...] a return from the initial call to the main function is equivalent to calling the exit function with the value returned by the main function as its argument; reaching the } that terminates the main function returns a value of 0.

For C++, since the first standard in 1998; see ISO/IEC 14882:1998 section 3.6.1:

If control reaches the end of main without encountering a return statement, the effect is that of executing return 0;

All versions of both standards since then (C99 and C++98) have maintained the same idea. We rely on automatically generated member functions in C++, and few people write explicit return; statements at the end of a void function. Reasons against omitting seem to boil down to "it looks weird". If, like me, you're curious about the rationale for the change to the C standard read this question. Also note that in the early 1990s this was considered "sloppy practice" because it was undefined behavior (although widely supported) at the time.

So I advocate omitting it; others disagree (often vehemently!) In any case, if you encounter code that omits it, you'll know that it's explicitly supported by the standard and you'll know what it means.

Answer 2

• You're still using C-style structs (using typedef). This is all you need for it in C++:

  struct Date {
      // ...
  };
  

• It's not really clear what errorCheck() is supposed to do. I would expect it to print error messages and such, but it just returns a bool. Perhaps you could rename the function to something more relevant, or provide comments.

  Moreover, I wouldn't recommend this style for conditionals. It'll make maintenance a bit harder in the event that more lines of code need to be added for some conditionals.

  Consider something like this:

  if (start.year>end.year)
  {
      return true;
  }
  else if (start.month > end.month && start.year == end.year)
  {
      return true;
  }
  else if (start.day > end.day && start.month == end.month && start.year == end.year)
  {
      return true;
  }
  
  return false;
  

• Use std::getline() for std::string input, not std::cin:

  std::getline(std::cin, startDayDate);
  

  std::getline(std::cin, endDate);
  

• Print errors to std::cerr, not std::cout.

• I don't have any specific ideas on hand, but there should be a simpler way of splitting a string by a delimiter. You can look here to get some ideas on what you would prefer.

Answer 3

The approach you use makes it hard to verify.

In my opinion the better approach to this problem is to use an epoch and calculate a day number for both start and end then simply subtract the day numbers to get the days between them.

The following use the "Gregorian" Calander which was valid from 1752 (In Great Britain and its colonies including the US). Other parts of the world switch at other times.

So I have written this to work with dates from 1/Jan 1753.

#include <iostream>
#include <string>
std::string getDate(std::string const& note)
{
    std::cout << "Please input " << note << ". Format (DD/MM/YYYY)\n";

    std::string result;
    std::cin >> result;
    // Do your validation here: including date >= 1753
    return result;
}
bool isLeapYear(std::size_t year)
{
    return year % 400 == 0 ||
           (year % 100 != 0 && year % 4 == 0);
}
std::size_t countLeapYears(std::size_t year)
{
    // 438 - 17 + 4
    static const std::size_t fakeLeaps = (1753/4) - (1753/100) + (1753/400);
                                              // Leaps before 1753
                                              // We start at 0 in 1753
    std::size_t leaps      = year/4;
    std::size_t badLeaps   = year/100;
    std::size_t extraLeaps = year/400;

    return leaps - badLeaps + extraLeaps - fakeLeaps;
}
std::size_t getDayId(std::string const& date)
{
    static const std::size_t monthInfo[2][12] = {{0,31,59,90,120,151,181,212,243,273,304,334}, {0,31,60,91,121,152,182,213,244,274,305,335}};
    std::size_t year = std::stoi(date.substr(6));
    std::size_t month= std::stoi(date.substr(3,2));
    std::size_t day  = std::stoi(date.substr(0,2));

    std::size_t noYears     = year - 1753;
    std::size_t leapYears   = countLeapYears(year);
    std::size_t daysInYear  = monthInfo[isLeapYear(year)][month - 1];

    std::size_t id = noYears * 365 + leapYears + daysInYear + (day - 1);
    return id;
}

Then to use:

int main()
{
    std::string st = getDate("The start date");
    std::string ed = getDate("The end date");

    std::size_t start = getDayId(st);
    std::size_t end   = getDayId(ed);

    std::cout << "Number of days: " << (end - start) << "\n";
}