C++

Forum

 
[Date Class] Question/Review

Misenna (128)
I am currently writing my first class. The program is supposed to get month, day, and year information, which is validated by member functions, then output to screen. All seems to work just fine, no errors or such.

Yet, I'm not sure whether I got the class design right, or what I could/should do differently. Specifically the array in my specification file. Is this done right? Comments, suggestions and critique would be most welcome!

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
 
/* Date.h - Specification file for the Date class. */

#pragma once
#ifndef DATE_H
#define DATE_H

class Date
{
	private:
		int month;		/* Holding the month */
		int day;			/* Holding the day   */
		int year;		/* Holding the year  */

		static const std::array<int, 13> daysPerMonth;			/* Holds number of days per month */
		static const std::array<std::string, 12> monthNames;	/* Holds month names (JAN -> DEC) */

	public:		
		bool setMonth(int);
		bool setDay(int);
		bool setYear(int);

		void displayFormatOne() const;
		void displayFormatTwo() const;
		void displayFormatThree() const;
};
#endif


The Implementation:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
 
/* Date.cpp - Implementation file for the Date class. */

#include <array>
#include <string>
#include <iostream>
#include "Date.h"

using std::array;
using std::string;
using std::cout;

/* Initializes the daysPerMonth array with the days per month */
const array<int, 13> daysPerMonth = { 0, 31, 28, 31, 30, 31, 30,
31, 31, 30, 31, 30, 31 };

/* Initializes the monthNames array with the names of the months of the year */
const array<string, 12> monthNames = { "JANUARY", "FEBRUARY", "MARCH", "APRIL",
 "MAY", "JUNE", "JULY", "AUGUST", "SEPTEMBER", "OCTOBER", 
"NOVEMBER", "DECEMBER" };

/* Enumerator to validate Month values */
enum Months { JANUARY = 1, FEBRUARY, MARCH, APRIL, MAY, 
              JUNE, JULY, AUGUST, SEPTEMBER, 
              OCTOBER, NOVEMBER, DECEMBER };

enum Years { LOWEST = 1950, HIGHEST = 2100 };

/* **********************************************************
				Date::setDay (accepts month)
	If the arguments passed to the setMonth function are equal
	to or greater than JANUARY and smaller than or equal to
	DECEMBER it is copied into the member variable month, and 
	true is returned. If it is not, the value of day remains 
	unchanged and false is returned.
   ********************************************************** */

bool Date::setMonth(int mm)
{
	bool isValidMonth = false;

	mm >= JANUARY && mm <= DECEMBER ? month = mm, isValidMonth = true : isValidMonth = false;
	
	return isValidMonth;
}

/* **********************************************************
				Date::setDay (accepts day member)
	If the argument passed to the setDay function is greater
	than 1 and less than daysPerMonth (validating that the day 
	entered is within range of any given month, ex: May = 31), 
	it is copied into the member variable day, and true is 
	returned. If it is not, the value of day remains unchanged 
	and false is returned.
   ********************************************************** */

bool Date::setDay(int dd)
{
	bool isValidDay = false;

	dd >= 1 && dd <= daysPerMonth[month] ? day = dd, isValidDay = true : isValidDay = false;

	return isValidDay;
}

/* **********************************************************
				Date::setYear (accepts year member)
	If the argument passed to the setYear function is greater
	than 1 and less than 2100, it is copied into the member
	variable 'year', and true is returned. If it is not, the
	value of year remains unchanged and false is returned.
   ********************************************************** */

bool Date::setYear(int yyyy)
{
	bool isValidYear = false;

	yyyy >= LOWEST && yyyy <= HIGHEST ? year = yyyy, isValidYear = true :  isValidYear = false;

	return isValidYear;
}

/* **********************************************************
				Date::displayFormatOne 
	This function displays the date as 12/12/2012
   ********************************************************** */

void Date::displayFormatOne() const
{
	cout << "\nDate Format One:\n";
	cout << month << "/" << day << "/" << year << "\n\n";
}

/* **********************************************************
				Date::displayFormatTwo 
	This function displays the date as DECEMBER 12, 2012
   ********************************************************** */

void Date::displayFormatTwo() const
{
	cout << "Date Format Two:\n";
	cout << monthNames[month - 1] << " " << day << ", " << year << "\n\n";
}

/* **********************************************************
				Date::displayFormatThree 
	This function displays the date as 12 DECEMBER, 2012
   ********************************************************** */

void Date::displayFormatThree() const
{
	cout << "Date Format Three:\n";
	cout << day << " " << monthNames[month - 1] << " " << year << "\n";
}


The main.cpp:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
 
#include <iostream>
#include <string>
#include "Date.h"

using std::cin;
using std::cout;

void getCurrentDate(Date &);

int main()
{
	/* Declare a Date object */
	Date date;

	getCurrentDate(date);

	/* Display the dates */
	date.displayFormatOne();
	date.displayFormatTwo();
	date.displayFormatThree();

	cin.get();
	cin.ignore();
   return 0;
}

/* **********************************************************
	Definition: getCurrentDate 
	
	Accepts a Date class object passed to it by reference. The
	user is asked to enter a month, day, and year. If valid,
	this information is stored in the class object's member
	variables.
	********************************************************** */

void getCurrentDate(Date &date)
{
	int month = 0;
	int day = 0;
	int year = 0;

	cout << "Enter a month: ";
	cin >> month;

	/* Call member function to set month member variable. If the
	function call returns false, the user is asked to repeat
	the input. */
	while (date.setMonth(month) == false)
	{
		cout << "\nYou entered an invalid month.\n"
			  << "Enter a month: ";
		cin >> month;
	}

	cout << "Enter a day: ";
	cin >> day;

	/* Call member function to set day member variable. If the
	function call returns false, the user is asked to repeat
	the input. */
	while (date.setDay(day) == false)
	{
		cout << "\nYou entered an invalid day.\n"
			  << "Enter a day: ";
		cin >> day;
	}

	cout << "Enter a year: ";
	cin >> year;

	/* Call member function to set year member variable. If the
	function call returns false, the user is asked to repeat
	the input. */
	while (date.setYear(year) == false)
	{
		cout << "\nYou entered an invalid year.\n"
			  << "Enter a year: ";
		cin >> year;
	}
}
Last edited on
JLBorges (13770)
You may want to handle the case where the year is a leap year (and February has 29 days).

Consider providing default member initialisers so that a default-initialised Date would be in a valid state.
http://www.stroustrup.com/C++11FAQ.html#member-init

Consider making the month an enumeration (it can take only one of the valid values).

Something like this:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
 
class Date
{
    public:

        // ensures that the month field would always be valid
        // usage: dt.set_month( Date::MARCH ) ;
        enum month_t { JANUARY = 1, FEBRUARY, MARCH, APRIL, MAY, JUNE, JULY,
                       AUGUST, SEPTEMBER, OCTOBER, NOVEMBER, DECEMBER };

        // this should be part of the public interface
        // (document the valid range)
        static constexpr int LOWEST_YEAR = 1950 ;
        static constexpr int HIGHEST_YEAR = 2100 ;

        bool set_year( int y ) ;
        void set_month( month_t m ) { month = m ; } // always valid
        bool set_day( int d ) ;

	void displayFormatOne() const;
	void displayFormatTwo() const;
	void displayFormatThree() const;

	private:

	// provide default member initialisers to initialise the object to a valid state
        month_t month = JANUARY ; // keep the month as an enumerated value
        int day = 1 ;
        int year = LOWEST_YEAR ;

        static bool is_leap( int year ) ; // return true if leap year

        static int days_in_month( month_t month, int year )
        {
            int ndays = daysPerMonth[month] ;
            if( month == FEBRUARY && is_leap(year) ) ++ndays ;
            return ndays ;
        }

        static const std::array<int, 13> daysPerMonth;       /* Holds number of days per month */
        static const std::array<std::string, 12> monthNames; /* Holds month names (JAN -> DEC) */
};

bool Date::set_year( int y )
{
    if( y < LOWEST_YEAR || y > HIGHEST_YEAR ) return false ;
    year = y ;
    return true ;
}

bool Date::set_day( int d )
{
    if( d < 1 || d > days_in_month( month, year ) ) return false ;
    day = d ;
    return true ;
}

Last edited on
tpb (1495)
Does it make sense to use "pragma once" AND include guards at the same time? I'm not sure. The only possible reason I can imagine is for "efficiency". "pragma once" is more efficient, but if the compiler doesn't recognize it then the include guards will do the job anyway. Is that your thinking?

You need to include <array> in Date.h.

You should use single line comments where appropriate.

You forgot to put Date:: in front of daysPerMonth and monthNames when defining them. (Did you try compiling this?)

Don't use the conditional operator to emulate an if statement. You've already initialized isValidWhatever to false anyway, so you don't even need an else part. You don't even need isValidWhatever.
1
2
3
4
5
6
7
 
bool Date::setYear(int y)
    if (y >= LOW_YEAR && y <= HIGH_YEAR) {
        year = y;
        return true;
    }
    return false;
}

Why isn't getCurrentDate in the Date class? It seems ... related.

The final cin.ignore() serves no purpose.

And what about Feb 29? Dealing with leap days is easy, but you'll need to read in the year and month before the day. If the month is FEBRUARY and the year is a leap year, then you need to allow the high value to be one greater.
1
2
3
 
bool Date::isLeapYear(int y) {
    return y % 400 == 0 || (y % 4 == 0 && y % 100 != 0);
}

Misenna (128)
JLBorges, thank you very much for the example code and your suggestions! I like your idea of having a valid month value, but don't think it should be part of the current project.

The problem description in my book says this:
The class should store a date in three integers, output it to screen three different ways, and:

"Input Validation: Do not accept values for the day greater than 31 or less than 1. Do not accept values for the month greater than 12 or less than 1.


Though this isn't necessary, I will definitely add the isLeap validation.
Misenna (128)
Does it make sense to use "pragma once" AND include guards at the same time? I'm not sure. The only possible reason I can imagine is for "efficiency". "pragma once" is more efficient, but if the compiler doesn't recognize it then the include guards will do the job anyway. Is that your thinking?

Yes, this is what I was thinking. If one or the other is preferred, I would stick to one or the other.

You need to include <array> in Date.h.

It was in there originally, then I tried without. Not having it in the Date.h file didn't seem to make a difference, no warnings, no errors. I added it back in just in case.

You should use single line comments where appropriate.

Your remark on the comments, maybe I should use single line comments, or should start using them? To be honest, I haven't put much thought into it. I always thought that it is a matter of convenience and personal style which to use - // or /**/ all the way or a mixture of both.

You forgot to put Date:: in front of daysPerMonth and monthNames when defining them. (Did you try compiling this?)

I haven't forgotten it. I tried with and without, in both cases it worked just fine in VS. I added it back in when I saw that it wouldn't work as expected in the cpp-shell.

Don't use the conditional operator to emulate an if statement. You've already initialized isValidWhatever to false anyway, so you don't even need an else part. You don't even need isValidWhatever.

A case of 'stupid choice'. I originally had an if/else statement in place of ternary operators. Then I saw in my reference book that it would also be possible to use it instead, now seeing that it only leads to more code than is necessary in this particular case. I changed it (back) as per your suggestion.

Why isn't getCurrentDate in the Date class? It seems ... related.

Yes, it is indeed. So, would say it is better to have it like this?

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
 
void Date::getDate()
{
	cout << "Enter a month: ";
	cin >> month;
       
        // should this be prefixed with Date? Meaning while (Date::setMonth(month) or is the other version also acceptable?
	while (setMonth(month) == false)
	{
		cout << "\nYou entered an invalid month.\n"
			  << "Enter a month: ";
		cin >> month;
	}

	cout << "Enter a day: ";
	cin >> day;

	while (setDay(day) == false)
	{
		cout << "\nYou entered an invalid day.\n"
			<< "Enter a day: ";
		cin >> day;
	}

	cout << "Enter a year: ";
	cin >> year;

	while (setYear(year) == false)
	{
		cout << "\nYou entered an invalid year.\n"
			  << "Enter a year: ";
		cin >> year;
	}
}


The final cin.ignore() serves no purpose.

It keeps the console window open. Should be the other way around, though. Could as well be ignore twice or (bad idea: cin.get() twice. This can lead to unexpected problems even though it does do the same thing: keep the console open.)
Topic archived. No new replies allowed.