tags:

views:

131

answers:

4

For example:

void Date::month(unsigned int inMonth) {
    assert(inMonth <= 12);
    _month = inMonth;
}

If this is not good practice, what is the correct way to go about this?

A: 

Yes it is.

This is the right way.

Although I would call it setMonth() rather than month()

Also bare in mind that assert() is preprocessed into nothing in release builds. so if you want something that also works in release then either write your own assert or do a proper run-time check.

shoosh
A: 

That's fine, you can assert whenever it's unexpected input in your application. In addition throw an exception something like ArgumentException in addition since you don't want to set an invalid month value.

Yuval Peled
+1  A: 

Another way that also gives the code more readability would be defining an enumerated type for month:

enum e_Month {
   e_Month_January,
   e_Month_February, 
   e_Month_March, 
   // etc..
   e_Month_December
};

Now your assignment becomes:

void Date::month(e_Month inMonth) { _month = inMonth; }

Most compilers will cause an error for assigning another type to enum so you get compile time safety that it will always be in range.

MadcapLaugher
+2  A: 

You shouldn't use assert to ensure that an argument to a public member function is valid. The reason is that there's no way for clients of your class to react to a failed assertion in any meaningful way (the fact that asserts are removed from release builds does not help either).

As a rule of thumb, assert should only be used for things that are strictly under your control (e.g., an argument of a private method).

In this case you're better off throwing a std::invalid_argument exception:

void Date::month(unsigned int month) 
{
    if(month == 0 || month > 12)
    {
        throw std::invalid_argument("a month must be in the [1-12] range"); 
    }
    _month = month;
}
Manuel