Date class implementation in C++

Question

Here is my implementation of a Date class.

I am interested mainly in the class implementation and design, whether or not it has any major drawbacks and inefficiencies. But I will appreciate any given recommendations, warnings and advises.

The creation of the class is only for learning purposes, production usage is not intended.

Date.h:

#pragma once
#ifndef _DATE_H_
#define _DATE_H_

#include <iostream>
#include <string>

class Date {
public:
    struct BadDate;
    enum Month {
        jan = 1, feb, mar, apr, may, jun, jul, aug, sep, oct, nov, dec
    };

private:
    static std::string monthNames [12];
    static bool defaultSet;
    static Date defaultDate;

    int _day, _month, _year;
    bool leapYear(const int) const;
    void fillDate(int d, Month m, int y);
    void checkIllFormed();
    bool isIllFormed() const;
    void setVal(int&, const int, const int);

public:
    static void setDefault(const int d = 1, Month = Month(1), const int y = 2000);
    static void showDefault(std::ostream& os);
    static const std::string monthNameByNumber(const int);

    Date(
        int d = (Date::defaultSet)?Date::defaultDate.day():1,
        Month m = (Date::defaultSet)?Date::defaultDate.month():Month(1),
        int y = (Date::defaultSet)?Date::defaultDate.year():2000
    );
    Date(int d, int m, int y);
    Date(const Date&);
    Date& operator=(const Date&);
    ~Date();

    int day() const;
    Month month() const;
    int year() const;

    const std::string getMonthName() const;

    void setDay(const int);
    void setMonth(const int);
    void setYear(const int);

    const Date& operator++();
    const Date  operator++(int);
    const Date& operator--();
    const Date  operator--(int);
};

struct Date::BadDate {
    int _day, _month, _year;
    BadDate(int d, int m, int y);
};

std::ostream& operator<<(std::ostream&, const Date&);
std::ostream& operator<<(std::ostream&, const Date::BadDate&);

#endif

Date.cpp:

#include "Date.h"

Date::BadDate::BadDate(int d, int m, int y)
    : _day(d), _month(m), _year(y) {

};

std::string Date::monthNames[12] = {
    "January", "February", "March",
    "April", "May", "June",
    "July", "August", "September",
    "October", "November", "December"
};
bool Date::defaultSet = true;
Date Date::defaultDate = Date(1, 1, 2000);

void Date::setDefault(const int d, Month m, const int y) {
    Date::defaultDate.setDay(d);
    Date::defaultDate.setMonth(m);
    Date::defaultDate.setYear(y);
    Date::defaultSet = true;
}
void Date::showDefault(std::ostream& os) {
    os << Date::defaultDate;
}
const std::string Date::monthNameByNumber(const int n) {
    return Date::monthNames[n-1];
}

Date::Date(int d, Month m, int y)
    : _day(d), _month(m), _year(y) {
        checkIllFormed();
}
Date::Date(int d, int m, int y)
    : _day(d), _month(Month(m)), _year(y) {
        checkIllFormed();
}
Date::Date(const Date& that)
    : _day(that.day()), _month(that.month()), _year(that.year()) {
        checkIllFormed();
}
void Date::checkIllFormed() {
    if(isIllFormed()) {
        BadDate bd = BadDate(day(), month(), year());
        *this = Date::defaultDate;
        throw bd;
    }
}
Date& Date::operator=(const Date& that) {
    fillDate(that.day(), that.month(), that.year());
    return *this;
}
Date::~Date(void) {

}

int Date::day() const {
    return _day;
}
Date::Month Date::month() const {
    return Month(_month);
}
int Date::year() const {
    return _year;
}

const std::string Date::getMonthName() const {
    return Date::monthNameByNumber(month());
}

void Date::setDay(const int d) {
    setVal(_day, d, day());
}
void Date::setMonth(const int m) {
    setVal(_month, m, month());
}
void Date::setYear(const int y) {
    setVal(_year, y, year());
}
void Date::setVal(int& val, const int newVal, const int prevVal) {
    val = newVal;
    if(isIllFormed()) {
        BadDate bd = BadDate(day(), month(), year());
        val = prevVal;
        throw bd;
    }
}

const Date& Date::operator++() {
    setDay(day() + 1);
    return *this;
}
const Date Date::operator++(int) {
    setDay(day() + 1);
    return *this;
}
const Date& Date::operator--() {
    setDay(day() - 1);
    return *this;
}
const Date Date::operator--(int) {
    setDay(day() - 1);
    return *this;
}

std::ostream& operator<<(std::ostream& os, const Date& d) {
    os << d.day() << '.' << d.getMonthName() << '.' << d.year();
    return os;
}
std::ostream& operator<<(std::ostream& os, const Date::BadDate& bd) {
    os << bd._day << '.' << Date::monthNameByNumber(bd._month) << '.' << bd._year;
    return os;
}

bool Date::leapYear(const int y) const {
    /*
    1.If the year is evenly divisible by 4, go to step 2. Otherwise, go to step 5.
    2.If the year is evenly divisible by 100, go to step 3. Otherwise, go to step 4.
    3.If the year is evenly divisible by 400, go to step 4. Otherwise, go to step 5.
    4.The year is a leap year (it has 366 days).
    5.The year is not a leap year (it has 365 days).
    */
    if(y%4) {
        return false;
    }
    if(y%100) {
        return true;
    }
    if(y%400) {
        return false;
    }
    return true;
}
void Date::fillDate(int d, Month m, int y) {
    setDay(d);
    setMonth(m);
    setYear(y);
}

bool Date::isIllFormed() const {
    const int d(day()), m(month()), y(year());

    //check year
    if(y < 0) {
        return true;
    }

    //check month
    if(m < 1 || 12 < m) {
        return true;
    }

    //check day
    int maxDay((7<m)?31-m%2:30+m%2);
    if(m == 2) {
        if(leapYear(y)) {
            maxDay = 29;
        } else {
            maxDay = 28;
        }
    }
    if(d<1 || maxDay<d) {
        return true;
    }

    return false;
}

I saw this other question, but my Date class is different and may have its own pitfalls which I am not aware of and I would like to know.

Answer 1

1. Header guards

#pragma once
#ifndef _DATE_H_
#define _DATE_H_
#endif

The "classic" header guard and #pragma once are redundant. Use either one of them. If your code should be maximum portable, remove the #pragma once it's not supported by all compilers, while the #ifndef #define #endif sequence is.

Also note that symbols starting with a prefixed _ are reserved for compiler implementation internals in c++.

2. Unnecessary scope qualifying

Statements like

 Date::defaultDate.setDay(d);

inside of any class member functions don't need to qualify the scope of the class member unless you need to qualify an inherited class member or such.

You can simply write

 defaultDate.setDay(d);

there.

3. Correct use of const

The use of const in this function definition

const std::string Date::monthNameByNumber(const int n) {
     return Date::monthNames[n-1];
}

is wrong, or at least doesn't have any benefical effect.

It should rather be

const std::string& Date::monthNameByNumber(int n) const {
                                               // ^^^^^ No change of Date's
                                               //       internal state is
                                               //       guaranteed.
     return Date::monthNames[n-1];
}

or made a static class member function at all:

static const std::string& monthNameByNumber(unsigned int n) {
    static const std::string monthNames[] = {
        "January"s, "February"s, "March"s,
        "April"s, "May"s, "June"s,
        "July"s, "August"s, "September"s,
        "October"s, "November"s, "December"s
    };
    if(n >= (sizeof(monthNames)/sizeof(std::string))) {
         std::ostringstream msg;
         msg << "Valid range of n is [0-" << sizeof(monthNames)/sizeof(std::string) << "]";
         throw std::out_of_range(msg.str());
    }
    return monthNames[n];
}

4. Avoid unnecessary casts

You have declared an enum type for the representation of months, but then you prefer to cast instead of just using it:

Month = Month(1),

That should be

Month = jan,

Casting there defeats the whole purpose of using enum values to enhance type safety and readability of the code.

5. Use unsigned for values always greater or equal zero

Here you never expect negative values

int _day, _month, _year;

so these variables should be declared as unsigned:

unsigned int _day, _month, _year;

Also avoid multiple variable definitions in a single line for sake of clarity and readability:

unsigned int day_;
unsigned int month_;
unsigned year_;

_{Note I moved the _ to the end of the names, same reasoning as stated in my point 1.}

6. Getter/Setter naming style

You should improve the naming style of your getter/setter functions. These can be just overloaded function signatures:

 class Date {
     unsigned int day_;
     // ...
 public:
     unsigned int day() const;         // Getter signature
     unsigned int day(unsigned int);   // Setter signature (returns the old value)
 };

The standard library uses that style a lot, have a look at e.g. the stream flag getters and setters.

7. Use std::exception descendants when throwing

When ever you are going to throw exceptions use either std::exception or a class inheriting it.

So instead of

    BadDate bd = BadDate(day(), month(), year());
    val = prevVal;
    throw bd;

you should create a class

 class BadDateException : public std::exception {
     std::string message;
     Date badDate;
 public:
     BadDateException(const Date& d) : badDate(d) {
         std::ostringstream msg;
         msg << "Date " << d << " isn't a valid date.";
         message = msg.str();
     }
     const char* what() {
         return message.c_str();
     }
 }

thus the clients can handle exception in a more generic way and don't need to know about your BadDate class:

 try {
     Date d;
     std::cin >> d;
 }
 catch(const std::exception& e) {
     std::cerr << "Caught exception '" << e.what() << "'" << std::endl;
     exit(1);
 }

If one needs to handle your exception specifically it can be done like

 try {
     Date d;
     std::cin >> d;
 }
 catch(const BadDateException& e) {
     std::cerr << "The date value is invalid '" << e.what() << "'" << std::endl;
     exit(42); // Do something specific for that exception
 }
 catch(const std::exception& e) {
     std::cerr << "Caught exception '" << e.what() << "'" << std::endl;
     exit(1);
 }

8. Follow the principle of least astonishment

Your way to pre-configure the default date violates the POLA policy.

I'd leave that feature out completely. It's way more readable and clear to use something like

const Date defaultDate(1,Jan,200);

Date d;
if(!std::cin >> d) {
    d = defaultDate;
}

or

const Date defaultDate(1,Jan,200);
Date d(defaultDate);

in your local context.

Someone could have used setDefaultDate() elsewhere in a different context, and the client is wondering why simply declaring Date d; uses those specific values.

Answer 2

1. Header Names

I recommend using .hpp for purely C++ header files, and reserving .h to C-compatible header files.

2. Header Guards

As mentioned, you should #pragma once rather than #ifndef.

If you really wish to use the error prone header guards; then I recommend following the Boost convention for naming: PROJECT_PATH_FILE_INCLUDED which helps prevent clashes in header guard names. It's significantly more work, of course...

3. Namespace

By defining symbols in the global namespace, you open your code up to conflicts with C libraries, or other uncouth C++ libraries.

Instead, decide on a namespace, generally the project name, and wrap any single item you define in this namespace.

4. Use enum class, and pick an underlying type.

Any new enum should really be enum class, which introduces a scope for the enumerators rather than injecting them in the surrounding scope.

Since you are no longer polluting the surrounding namespace, you can without fear pull out Month at namespace level, making it easier for people to name the type.

I also encourage you to pick a type. By default the compiler will use int, which is... 3 bytes too wide in your case. You can easily use int8_t instead, it is large enough.

Also, you might as well dispense with pointless abbreviations.

This gives:

#include <cstdint>

namespace mydate {
enum class Month: std::int8_t {
    January = 1, February, March, April, May, June,
    July, August, September, October, November, December,
};
}

As a side bonus, enum class can be forward declared by your users.

5. Exception should inherit std::exception.

It's a convention that most everyone follows which allows catching all exceptions and being able to display something with .what() which beats ... hands down.

6. Public first, please.

Header files are often peeked at by your clients.

Therefore, it is a good idea to expose your class interface in the order: public, protected, private. This minimizes the number of irrelevant lines that the user has to skip.

7. There shall be no global state.

Global state is a plague on software as it prevents local reasoning, the primary tool you have to understand how a piece of software works.

It also suffers from data-races, however just slapping thread_local is not a panacea either: it doesn't solve the local reasoning issue and prevents clients from using different threading models such as coroutines.

In your case, it's even worse, because there is no good default date which will satisfy every user.

8. Global constants are good.

But they shall be constants, hence:

static std::array<std::string, 12> const monthNames;

Using std::array is preferred. You get some goodies, such as bounds checking, easy extraction of size, etc...

9. Pick your types.

int is a default type, but it is not very sensible.

You already have a Month enum, why is not used for _month?

I also recommend using a Day type, or at least using std::int8_t again since there's no reason to have a day greater than 127.

10. Stateless helper functions.

The leapYear function does not access this, so it need not be a class method.

You can either propose it as a free-function as part of your interface, or otherwise only declare it in the .cpp file in an anonymous namespace.

11. Never return a const value.

It's rather pointless, as the client can copy it anyway.

You may wish to return a T const& (const-reference). Unfortunately it means that clients get to know that your class contains a T, so that if you ever want to change the internals of your class, clients will break.

12. Defaults are hard.

If there is no meaningful default value for your type, consider NOT defining a default constructor.

This is pretty restrictive, of course, so you may consider instead:

• defining an invalid value, though this means that (1) your class must now have a isValid method and (2) on each getter your class must check whether it is valid.
• defining a default value nonetheless, and document it. In this case, it may be a good to pick an unlikely value, such as the 1st January of the year 1, to help clients realize when they forgot to set the value.

13. Beware typeless interfaces.

The US format for dates is MM/DD/YYYY, the ISO format is YYYY/MM/DD. When you present a constructor Date(int, int, int), it's very easy for a client to accidentally pass the arguments in the wrong order.

In this sense, Date(int, Month, int) is a step up, since Americans cannot accidentally use their weird format, however it's still possible to accidentally use ISO instead of DD/MM/YYYY as you intend.

There are 2 possibilities out of there:

• Only offer strongly typed arguments: Date(Day, Month, Year),
• Use named constructors instead.

The latter is done with static methods:

static Date from_date_month_year(int day, Month month, int year);

Note: while it's relatively clear that int day will have a range [1, 31], it's unclear whether int month has a range [0, 11] or [1, 12]; don't tempt the user.

14. Name your parameters.

Here, d is probably understood, but in general please attempt to give your parameters full names. Abbreviations are really not necessary in general.

15. Rule of Zero

The Rule of Zero states that you should rarely (if ever) define a Copy Constructor, Copy Assignment Operator, Move Constructor, Move Assignment Operator or Destructor.

It is only necessary on technical resource management classes, such as unique_ptr or vector. If you ever find yourself writing such a class, be sure to make it as minimal as possible; without any business logic.

16. Be consistent.

Your interface sometimes use int for month (where it's ambiguous) and sometimes Month. Stick to Month.

17. Follow conventions.

The return type of operator++ and operator++(int) is never const-qualified, to allow chaining.

18. Be sparse in your includes.

You never use ostream, so you may as well only include <iosfwd> in your header.

Minimizing the amount of includes in your headers help with compilation times.

---

I'll stop here, as that's a lot to fix already.

Congratulations on writing your Date class :)

Answer 3

[code] header guard

Personally I'd recommend just using #pragma once. It's supported by most compilers, it's less typing, less work for the preprocessor, and much less error prone (renaming files and forgetting to rename the define, issues with reserved names, typos in the definition / check, etc.)

[design] default

The static default functionality probably shouldn't be inside the class as it's error prone and assumes the users needs (e.g. What if they need two different defaults? They'd have to keep changing it backwards and forwards, which makes it pointless).

If the user wants a default value, they can store a constant Date instance somewhere themselves, and pass that it into the constructor easily enough. This also makes it instantly obvious what value an intialized object is:

const Date defaultDate(23, Date::Month::mar, 3);

Date d1; // no idea what value this has... it depends what the default is at the moment! we now have to find out if this is set anywhere else in the codebase, AND then understand which code paths lead from there to here...
Date d2(defaultDate); // it's set to the value of defaultDate!

[design] checking values

Presumably, the idea is to prevent creation of an invalid date, and have a transactional interface (i.e. changing the date either works, or throws an exception, leaving the current value untouched.

(Bug:) However, the way fillDate is currently defined, we try to set each component individually, then check if the whole date object is still valid at each step. This may cause problems, because an intermediate date may be valid (and so we successfully change the day or month), but not the final date. This means that although we throw an error, the initial date value has still been changed.

Furthermore, setting a value, then checking, then setting it back again and throwing is inefficient.

Instead we could just pass the numbers to be checked to a checking function (i.e. isIllFormed) before setting anything. This can also simplify construction and assignment:

Date::Date(): // default constructor provided
    _day(1), _month(1), _year(2000) { }

Date::Date(int d, int m, int y)
    : Date() { // use delegating constructor then call a single point of checking / setting

    set(d, m, y);
}

Date::Date(const Date& that)
    : Date() {

    set(that.day(), that.month(), that.year());
}

Date& Date::operator=(const Date& that) {

    set(that.day(), that.month(), that.year());

    return *this;
}

void Date::set(int day, int month, int year)
{
    if (!isValidDate(day, month, year)) // this check and the throw could be perhaps abstracted into a throwIfInvalid function...
        throw BadDate(day, month, year);

    _day = day;
    _month = month;
    _year = year;
}

void Date::setDay(const int d) {

    if (!isValidDate(d, month(), year())) 
        throw BadDate(d, month(), year());

    _day = d;
}
void Date::setMonth(const int m) {

    if (!isValidDate(day(), m, year()))
        throw BadDate(day(), m, year());

    _month = m;
}
void Date::setYear(const int y) {

    if (!isValidDate(day(), month(), y))
        throw BadDate(day(), month(), y);

    _year = y;
}

// isValidDate is just isIllFormed with the logic reversed

isValidDate and isLeapYear can be static functions, and might as well be public, since they'd be useful for users who need to validate input before creating a date class:

static bool isValidDate(int d, int m, int y);
static bool isLeapYear(int y);

Answer 4

In addition to the many points raised in the 3 existing answers, there are still a lot of other issues in that design.

Assignment

1. Operator = is inefficient. If validation was properly done with exception so that it is impossible to create an invalid date, then you won't need validation… And even if you want validation, you don't need to do it 3 times.

2. Individual functions like setDay, setMonth and setYear does not make much sense for a date. You almost always want to set all 3 components at once. And for the very few times you would want to update only some components, you can always read parts that do no not change.

Construction

3. Constructor order is unusual. Almost everyone would assume that the order is year, month and day and doing otherwise would only cause many bugs.

4. Your customizable default value does not make much sense. Also, it is not thread safe. If you need some predefined values, you make some constants. Default date would usually be the minimum supported date (for ex. 0001-01-01).

5. Also, you should not have default values for each parameter on the constructor taking an int, a Month and another int. The default would very rarely be appropriate so if someone create a date with some defaults, then the code would be harder to understand. Also as your constructor is not explicit, it can lead to undesirable conversion constructor being called in some contexts.

Increment and decrement operators

6. Your postfix operators do not returns the old value as expected. As already mentioned by others, return type should not be const. You should almost always mimic what is done on predefined types.

7. Your operators does not properly handle month and year changes. If you provide such operators, then at least you should make sure that incrementing last day of a month will return first day of next month. And for decrementing, you should go to last day of previous month. And you probably also want to ensure that your date stays within the supported range.

8. It is recommended to code the postfix operator using the prefix operator to avoid code duplication. You can easily write something like:

   Date operator++(int)
   {
       Date result(*this); // Make a copy
       ++*this;            // Increment this object
       return result;      // Return old value
   }
   

Readability

9. Use spacing in expression. Your should have a space around every binary operator in an expression (and also after branching keywords). So for ex. if(y%4) should really be if (y % 4).

10. You should avoid short variable names like d. By writing day instead, the code is much easier to read.

11. You should not write void for empty parameter lists in C++ like you did in: Date::~Date(void). Even worse, your definition is inconsistent with your declaration.

Validation issues

12. If you try to modify multiple parts of a date, like going from 2017-03-31 to 2016-02-29, it would fail if you do validation after setting the day and month but not the year.

13. You should also use assertion in your code to verify that you get expected results or that the code is properly used.

Unit testing

14. Before writing a library like this, you should write unit tests. Many problems like incorrect postfix operator result can easily be detected with a properly written test.

Conclusion

Given the high number of issues (between 30 and 45 issues), I would recommend you to read a bunch of good C++ books before writing more "library" code. I highly recommend Meyers and Sutter books among others.

Also other libraries could be used as a source of inspiration even in other language like DateTime struct in .NET. In that case, value is immutable which is an even more valuable design.