C++

Forum

 
Where's the logic flaw?

RpTheHotrod (2)
Hey guys!

First of all, this is an awesome website. Very well designed and informative!

I'm new to C++ and I've been asked to create a program that gets user input in the form of MM-DD-YYYY and then output how many days it is in that year (also considering leap years).

I'm given no instruction on how to do so, but to figure it out. I'm not into arrays yet, and I've only learned about functions a few days ago.

Somewhere my logic is flawed in this and I can't quite see it at this point. The program "works", as far as I'm aware, but I'm going back and trying to validate the user input.

For ease's sake, 2008 is a leap year and 2009 is not.

I'm trying to lock out things such as days beyond 31, or 30 and 31 if feb, and 29 if it's not a leap year.

Can anyone see why I'm able to still put in invalid data at times? (February is a good starting point). Try 02-31-2009, 02-31-2008, 02-30-2009, 02-30-2008, 02-29-2009, 02-29-2008.






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
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
154
155
156
157
 
#include <iostream>

using namespace std;

int month, day, year;
int count = 0;
char dash;
bool evenDay = 0;

void badDate();
bool leapYearCheck();
bool initiallyGood();
bool badDay();

int main()
{
	
	cout << "Please enter a date in MM-DD-YYYY format." << endl;
	cin >> month;
	cin >> dash;
	cin >> day;
	cin >> dash;
	cin >> year;
	cout << endl;

	switch(month)
	{
		case 4:
		case 6:
		case 9:
		case 11:
			evenDay = 1;
			break;
		default:
			evenDay = 0;
			break;
	}

	// Testing initial date input
	while((initiallyGood() == 0) || (badDay() == 1))
	{
		badDate();
	}

	// Testing input
	//cout << month << " " << day << " " << year << endl;
	
	month = month - 1;
	
	/* Testing month change
	cout << "New Month: " << month << endl;
	*/

	switch(month)
		{
			case 1:
			//case 01:
				count = count + 31;
				break;
			case 2:
			//case 02:
				count = count + 59;
				break;
			case 3:
			//case 03:
				count = count + 90;
				break;
			case 4:
			//case 04:
				count = count + 120;
				break;
			case 5:
			//case 05:
				count = count + 151;
				break;
			case 6:
			//case 06:
				count = count + 181;
				break;
			case 7:
			//case 07:
				count = count + 212;
				break;
			case 8:
			//case 08:
				count = count + 243;
				break;
			case 9:
			//case 09:
				count = count + 273;
				break;
			case 10:
				count = count + 304;
				break;
			case 11:
				count = count + 334;
				break;

		}
	
	// If a leap year, add a day.
	if (leapYearCheck() == 1)
		{
			count = count + 1;
		}

	count = count + day;

	cout << "The number of days is " << count << endl;
	
	return 0;
}

void badDate()
{
	cin.clear();
	cin.ignore(100,'\n');
	cout << "Invalid date! Please enter a date in MM-DD-YYYY format (ie 02-15-1984)." << endl;
	cin >> month;
	cin >> dash;
	cin >> day;
	cin >> dash;
	cin >> year;
	cout << endl;

	switch(month)
	{
		case 4:
		case 6:
		case 9:
		case 11:
			evenDay = 1;
			break;
		default:
			evenDay = 0;
			break;
	}

}

bool leapYearCheck()
{
	if ((year % 4 == 0) && (year % 100 != 0) || (year % 100 == 0) && (year % 400 == 0))
		return 1;
}

bool initiallyGood()
{
	if((((month >= 1) && (month <= 12)) && ((day >= 1) && (day <= 31)) && ((year >= 1) && (year <= 9999))))
		return 1;
}

bool badDay()
{
	if(((day >= 1) && (day <= 31)) && (((day == 31) && (evenDay == 1)) || ((month == 2) && (day == 30)) || ((month == 2) && (day == 31)) || ((leapYearCheck() == 0) && ((month == 2) && (day == 29)))))
		return 1;
}
Disch (13742)
I'm not sure if this is the cause of your problems, but your functions always need to return something. Right now you're only returning '1' if the condition is met. If the condition is not met, you're not returning anything (the compiler will not automatically return 0 for you!)


I do have some other advice:

1) Your use of globals makes this hard to follow and error prone. Globals really are evil and you should avoid using them. I know they seem simpler but really they make your code more complicated.


2) You could use more useful functions. It takes time to get the jist of how to form functions, but I usually do it by breaking a job into tasks.

For example:

The task at hand is to validate a date, right? So I would make an isDateValid function that you give a date to (via parameters). It checks that date and returns true if valid, and false otherwise.

bool isDateValid(int month,int day,int year); Notice how you shouldn't rely on globals. Passing the data to the function as parameters is more clear and more dynamic/useful.


Now take that a step further. How do you determine if a date is valid? The month has to be between 1-12, and the day has to be less than or equal to the number of days in the month, right? So wouldn't it be useful to know how many days are in the month? Since that's useful to know, I'd write a int daysInMonth(int month,int year); function. (EDIT: or 'getDaysInMonth' if you want to stick with verbs per #5)


Next, how do you determine how many days are in a month? It's pretty simple for most months, but February is tricky. In order to know how many days are in Feb, you need to know whether or not you have a leap year. So there's another function that's useful: bool isLeapYear(int year);



3) Avoid negative boolean variables or functions. badDay isn't a good choice for a function because it's a negative. isGoodDay would be better. Keeping it positive avoids confusing code caused by double negatives if(!badDay()) // if not a bad day? . This is more of a stylistic thing, and it might sound like I'm nitpicking somewhat. But trust me.

4) Avoid arbitrary or unclear functions. A function should have a clear task and should be named appropriately to describe that task. initiallyGood is not very descriptive and probably shouldn't be a function.


5) Names of functions should generally be verbs (since they describe the task they're doing). isBadDay is better than badDay, for example (although isGoodDay is even better per #3). You should not name functions based on when they're called. For example badDate is a bad function name. Logically it's called when a bad date is entered, but it doesn't describe the purpose of the function.

What's more, badDate and badDay are very similarly named, yet they do very different things. Awfully confusing and error prone, wouldn't you say?
Last edited on
RpTheHotrod (2)
That is very sound advice and I agree 100%. I will definitely consider that in future programs. I was writing and as I went I was actually thinking of a lot of the same issues as you stated (ie negative functions).

As for globals, that's the only thing our class knows right now. We do not know how to use something located locally in another function (ie main). I hear they are evil, but it's a necessary one until I learn otherwise. I'll try looking into this on my own.

As for return 0 issue, that actually solved it. Thanks! :D Now to see if there are any other holes...

One correction I spotted. On leapyear check +1 addition, it should be

if ((leapYearCheck()) == 1 && (month != 0) && (month != 1))

that way it doesn't add one if it was in January or February (the extra day in February would be considered in DAY).

Here is my modified code. I do not have time to re-write functions, at the moment.

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
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
154
155
156
157
158
159
160
161
162
163
164
165
 
#include <iostream>

//Jared Parks

using namespace std;

int month, day, year;
int count = 0;
char dash;
bool evenDay = 0;

void badDate();
bool leapYearCheck();
bool initiallyGood();
bool badDay();

int main()
{
	
	cout << "Please enter a date in MM-DD-YYYY format." << endl;
	cin >> month;
	cin >> dash;
	cin >> day;
	cin >> dash;
	cin >> year;
	cout << endl;

	switch(month)
	{
		case 4:
		case 6:
		case 9:
		case 11:
			evenDay = 1;
			break;
		default:
			evenDay = 0;
			break;
	}

	// Testing initial date input
	while((initiallyGood() == 0) || (badDay() == 1))
	{
		badDate();
	}

	// Testing input
	//cout << month << " " << day << " " << year << endl;
	
	month = month - 1;
	
	/* Testing month change
	cout << "New Month: " << month << endl;
	*/

	switch(month)
		{
			case 1:
			//case 01:
				count = count + 31;
				break;
			case 2:
			//case 02:
				count = count + 59;
				break;
			case 3:
			//case 03:
				count = count + 90;
				break;
			case 4:
			//case 04:
				count = count + 120;
				break;
			case 5:
			//case 05:
				count = count + 151;
				break;
			case 6:
			//case 06:
				count = count + 181;
				break;
			case 7:
			//case 07:
				count = count + 212;
				break;
			case 8:
			//case 08:
				count = count + 243;
				break;
			case 9:
			//case 09:
				count = count + 273;
				break;
			case 10:
				count = count + 304;
				break;
			case 11:
				count = count + 334;
				break;

		}
	
	// If a leap year, add a day if not in Janruary or February.
	if ((leapYearCheck()) == 1 && (month != 0) && (month != 1))
		{
			count++;
		}

	count = count + day;

	cout << "The number of days is " << count << endl;
	
	return 0;
}

void badDate()
{
	cin.clear();
	cin.ignore(100,'\n');
	cout << "Invalid date! Please enter a date in MM-DD-YYYY format (ie 02-15-1984)." << endl;
	cin >> month;
	cin >> dash;
	cin >> day;
	cin >> dash;
	cin >> year;
	cout << endl;

	switch(month)
	{
		case 4:
		case 6:
		case 9:
		case 11:
			evenDay = 1;
			break;
		default:
			evenDay = 0;
			break;
	}

}

bool leapYearCheck()
{
	if ((year % 4 == 0) && (year % 100 != 0) || (year % 100 == 0) && (year % 400 == 0))
		return 1;
	else
		return 0;
}

bool initiallyGood()
{
	if((((month >= 1) && (month <= 12)) && ((day >= 1) && (day <= 31)) && ((year >= 1) && (year <= 9999))))
		return 1;
	else
		return 0;
}

bool badDay()
{
	if(((day >= 1) && (day <= 31)) && (((day == 31) && (evenDay == 1)) || ((month == 2) && (day == 30)) || ((month == 2) && (day == 31)) || ((leapYearCheck() == 0) && ((month == 2) && (day == 29)))))
		return 1;
	else 
		return 0;
}


Last edited on
Topic archived. No new replies allowed.