My Calender Program

Did I write a good code?

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
#include <iostream>
#include <string>
using namespace std;
int main()
{
	int inc[12] = { 0, 3, 3, 6, 1, 4, 6, 2, 5, 0, 3, 5 };
	int y, m, d, t = 0, dx = 0;
	do
	{
		cout << "Enter Year";
		cin >> y;
	}
	while(y<1753);
	string w[7] = { "Mon", "Tue", "Wed", "Thu", "Fri", "Sat", "Sun" };
	int a, c;
	if (y>2013)
	{
		for (int i = 2013; i<y; i++)
		{
			if ((i % 4 == 0 && i % 100 != 0) || (i % 400 == 0))
				t = t + 1;
		}
		a = y - 2013;
		c = (1 + a + t) % 7;
	}

	if (y <= 2013)
	{
		for (int i = 2013; i >= y; i--)
		{
			if ((i % 4 == 0 && i % 100 != 0) || (i % 400 == 0))
				t = t + 1;
		}
		a = 2013 - y;
		c = (((1 - a - t) % 7) + 7) % 7;
	}
	do
	{
		cout << "Enter Month(1 to 12)";
		cin >> m;
	} while (m < 1 || 12 < m);
	while (dx != 1)
	{
		cout << "Enter day";
		cin >> d;
		if (((m == 1 || m == 3 || m == 5 || m == 7 || m == 8 || m == 10 || m == 12) && (0 < d &&d < 32)) || ((m == 4 || m == 6 || m == 9 || m == 11) && (1 <= d || d <= 30)) || ((m == 2) && ((0 < d&&d < 29) || (((y % 4 == 0 && y % 100 != 0) || (y % 400 == 0)) && (0<d&&d<30)))))
			dx = 1;
	}
	a = c + inc[m - 1];
	if ((2 < m) && (((y % 4 == 0) && (y % 100 != 0)) || (y % 400 == 0)))
		a = a + 1;
	for (int i = 0; i < 7; i++)
		cout << w[(a + i) % 7] << "\t";
	cout << endl;
	for (int j = 0; j < 4; j++)
	{
		for (int i = 1; i < 8; i++)
			cout << i + (7 * j) << "\t";
		cout << endl;
	}
	if ((m == 2) && ((y % 4 == 0 && y % 100 != 0) || (y % 400 == 0)))
		cout << "29" << endl;
	if (m == 1 || m == 3 || m == 5 || m == 7 || m == 8 || m == 10 || m == 12)
		cout << "29\t30\t31" << endl;
	if (m == 4 || m == 6 || m == 9 || m == 11)
		cout << "29\t30" << endl;
	cout << w[(a + d - 1) % 7] << " is the day on that day.\n";
	system("pause");
}
I would suggest you use meaningful and descriptive variable names and not single letters.

Also you can put statements across several lines so you don't have things like this: if (((m == 1 || m == 3 || m == 5 || m == 7 || m == 8 || m == 10 || m == 12) && (0 < d &&d < 32)) || ((m == 4 || m == 6 || m == 9 || m == 11) && (1 <= d || d <= 30)) || ((m == 2) && ((0 < d&&d < 29) || (((y % 4 == 0 && y % 100 != 0) || (y % 400 == 0)) && (0<d&&d<30)))))


Another thing you should generally stay away from using system for a number of reasons. The number one being it makes it so it is now platform dependent. Namespaces are meant for avoiding naming conflicts so when you put using namespace... that makes it so the namespace does absolutely nothing so you generally want to scope to the items in the namespace directly like: std::cout , std::string , std::endl; ect..
The big long statement that giblit mentioned could be avoided with some of these ideas:

1 A function for IsLeapYear have it return a bool
2 An array which holds the number of days in each month. Use the month number to look up the array
3 Use functions more to make the code more readable & aid understanding. And to make the code shorter - you have the logic on line 46 several times.

If you are going to use do loops, then put the while condition on the same line as the closing brace & put a blank line after.

What is the inc array for ? I see it's use on line 49, but am none the wiser. I am sure it is essential but I have no idea why, and am not going sit here for for 30 mins to see if I can guess.

When you write code, it is important that it be easy to understand by someone else - perhaps even if they don't know much about the subject matter. Use of functions with a meaningful name that says what it does plus a few lines of comments to specify preconditions like the range of valid values to be expected, and post conditions about what sort of value it will return. Also some comments pasted from the documentation (wiki for example), or a web address in a comment if that is going to help.

This is important because in industry, your code would be read by your supervisor, her/his boss, a maintenance programmer, or even yourself in 2 years time.

Some other things :

Always initialise your variables - preferably on the same line as declaration along with a comment if necessary. So declare 1 variable per line. Don't worry about making the code longer, rather that than it being hard to understand.

With arrays like on line 14, one can leave out the size of the array - the compiler will count how many there are.

t = t + 1 can be t++

Hope this helps & look forward to seeing your revised code :+)

EDIT:

With loops or any other control flow structure always use braces - even if it is 1 LOC like on line 53. This will save you one day when you add more code
Last edited on
inc array helps us find the first day of month once we know the first day of the year.
for example if the first day of the year is Friday and we want to know the first day of June then as June is the sixth month of the year we see the sixth element of the inc array. It's value is 4. We go 4 days ahead of Friday and we reach the first day of June which is Tuesday. In case of leap year and the month greater than 2 we go another day forward and reach Wednesday.
closed account (iAk3T05o)
You definitely need descriptive names. I didn't bother trying to understand the code because you have:
d, t, i, y, m, dx, inc, a, c.
And they can be anything.
@Agent

Thanks for the explanation - you have described your algorithm now in just a few lines - what would be great is if you had that as comments in your code. I made the comments so you could perhaps change your code so it is self explanatory.

Again we all look forward to seeing your revised code.

Cheers
Topic archived. No new replies allowed.