Difference in days between two dates

Question

I've been trying to write a really basic function to determine the difference (in days) between two dates. I am very new to C++ so as you can imagine I am very happy with my work. This function is part of a much larger code so I have missed off my includes, but the function does compile without warnings.

Just looking for some constructive criticism that a reasonable beginner can take on board.

I should add, this function takes two dates as strings in British format DD/MM/YYYY.

int dateDiffs(std::string date1, std::string date2){

    int startYear, endYear;
    int startMonth, endMonth;
    int startDay, endDay;

    int startInDays, endInDays;

    std::map<int,int> daysInMonth;

    daysInMonth[1]=31;
    daysInMonth[2]=28;
    daysInMonth[3]=31;
    daysInMonth[4]=30;
    daysInMonth[5]=31;
    daysInMonth[6]=30;
    daysInMonth[7]=31;
    daysInMonth[8]=31;
    daysInMonth[9]=30;
    daysInMonth[10]=31;
    daysInMonth[11]=30;
    daysInMonth[12]=31;

    startYear = atoi(date1.substr(6,4).c_str());
    startMonth = atoi(date1.substr(3,2).c_str());
    startDay = atoi(date1.substr(0,2).c_str());

    endYear = atoi(date2.substr(6,4).c_str());
    endMonth = atoi(date2.substr(3,2).c_str());
    endDay = atoi(date2.substr(0,2).c_str());

    startInDays = (startYear * 365) + (startMonth * daysInMonth[startMonth]) + startDay;
    endInDays = (endYear * 365) + (endMonth * daysInMonth[endMonth]) + endDay;

    return (endInDays - startInDays);
}

Answer 1

I see a number of problems with this:

• The code as provided does not compile; you haven't shown what #include directives you've used.
• You don't validate the input to make sure it is in the proper DD/MM/YYYY format.
• You don't validate the parsed year, month, and day to make sure it makes sense. (For instance, you allow negatives, and any month number or day number)
• You don't handle the case when the month number is outside the [1..12] range. This will lead to undesirable behavior (it returns 0 for the month length and expands the map)
• You assume all years have 365 days. This completely ignores leap years (366 days with February having 29 days)

And a few stylistic issues:

• This code should be part of a class rather than just a bare function
• Questionable data type choice. You use a std::map<> instead of a regular array (an int[]); this slows things down with the only benefit being that you assume the month length is 0 instead of referencing memory outside the array.
• Duplicated code (the date parsing). This should be abstracted out to a separate function (hopefully including the validation and checking)
• Unnecessary parens around the return value.
• You initialize the data table each time; this should be stored in a separate class so it can be created once and then referenced.

Answer 2

@Snowbody has addressed many important points. I'll just talk about the std::map.

The function initializes daysInMonth each time, which is just unnecessary. You should have it initialized only once elsewhere, and it should also be const since it's not to be modified within the code.

If you have C++11 (which you should), you can initialize the map with an initializer list:

const std::map<int, int> daysInMonth = { {1, 31},
                                         {2, 28},
                                         {3, 31},
                                         {4, 30},
                                         {5, 31},
                                         {6, 30},
                                         {7, 31},
                                         {8, 31},
                                         {9, 30},
                                         {10, 31},
                                         {11, 30},
                                         {12, 31} };

(it can be formatted in any way you'd like)

You should also use an unsigned type for dates since they cannot be negative.

Answer 3

I will not be marking this as the "answer to my question" but wanted to show how I have taken your advice on board:

I have written the following class:

unsigned int dayLimits[13] = {1, 31, 28, 31, 30, 31, 30, 31, 31, 30, 31, 30, 31};

class userDate {

    public:             
        unsigned int uYear, uMonth, uDay;

        bool valiDate(unsigned int uYear, unsigned int uMonth, unsigned int uDay){
            if ((uYear >= 2000) && (uYear < 2100)) {
                if ((uMonth > 0) && (uMonth <= 12)){
                    if((uDay >= dayLimits[0]) && (uDay <= dayLimits[uMonth])){
                        return true;
                    }
                }
            }

        };

}startDate, endDate;