Can my source code be condensed?

crestfallenDeus (5)
Hello world! This is my first discussion on this forum, so I'm a bit nervous. But could someone please help me revise the following program that I wrote for my C++ class? I guess what I'm really looking for is some positive criticism. I get the impression that my source code is a bit of an overkill for such a dippy program, so is there any way that I can improve upon it?

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
166
167
168
169
170
171
172
173
174
175
176
177
178
179
180
181
182
183
184
185
186
187
188
189
190
191
192
193
194
195
196
197
198
199
200
201
202
203
204
205
206
207
208
209
210
211
212
213
214
215
 
/*--------------------------------------------------------------------------------*/
/*                                                                                */
/*    ______++                                                                    */
/*   /_____/\       Name: David Bucio                                             */
/*   \:::__\/       Date: November 1, 2016                                        */
/*    \:\ \  __     Program: Days in a Month                                      */
/*     \:\ \/_/\    Summary: Debugging and Compiling Test                         */
/*      \:\_\ \ \                                                                 */
/*       \_____\/                                                                 */
/*                                                                                */
/*--------------------------------------------------------------------------------*/

/*--------------------------------------------------------------------------------*/
/*Identify Preprocessor Directives                                                */
/*--------------------------------------------------------------------------------*/
#include <iostream>
#include <fstream>
#include <string>

/*--------------------------------------------------------------------------------*/
/*Additional Requisites for Program                                               */
/*--------------------------------------------------------------------------------*/
using namespace std;

ofstream ofs ("daysPerMonth.txt");

/*--------------------------------------------------------------------------------*/
/*Initialize Global String Objects                                                */
/*--------------------------------------------------------------------------------*/
string absolutely = "David Bucio";
string no = "November 1, 2016";
string hard = "Days in a Month";
string coding = "CIS 2541";

string summary = "This program determines how many days are present in a month\n"
                 "with regards to a specific year.";

string note1 = "SOF Message: ";
string note2 = "EOF Message: ";

string gap1 = " ";
string gap2 = " | ";

string askMonth = "Enter a numeric month: ";
string askYear = "Enter a numeric year: ";

string showDays = "Days in given month: ";

/*--------------------------------------------------------------------------------*/
/*Initialize Global Variables                                                     */
/*--------------------------------------------------------------------------------*/
int month,
    year,
    day;

/*--------------------------------------------------------------------------------*/
/*Construct Header                                                                */
/*--------------------------------------------------------------------------------*/
void header()
{
	ofs << note1;
	ofs << absolutely << gap2 << no << gap2;
	ofs << hard << gap2 << coding;
	ofs << endl;
}

/*--------------------------------------------------------------------------------*/
/*Construct Footer                                                                */
/*--------------------------------------------------------------------------------*/
void footer()
{
	ofs << endl;
	ofs << summary;
	ofs << endl;
}

/*--------------------------------------------------------------------------------*/
/*End of File Message                                                             */
/*--------------------------------------------------------------------------------*/
void eofmsg()
{
	ofs << endl;
	ofs << note2;
	ofs << absolutely << gap2 << no << gap2;
	ofs << hard << gap2 << coding;
	ofs << endl;
}

/*--------------------------------------------------------------------------------*/
/*Program Input                                                                   */
/*   -Prompt user to enter both a month and year                                  */
/*--------------------------------------------------------------------------------*/
void getTime()
{
    cout << askMonth;
    cin >> month;
    cout << endl;
    
    cout << askYear;
    cin >> year;
    cout << endl;
}

/*--------------------------------------------------------------------------------*/
/*Program Calculation                                                             */
/*   -Help narrow down classification                                             */
/*--------------------------------------------------------------------------------*/
bool oddEven(int month)
{
    if (month % 2 != 0)
    {
        return false;
    }
    
    else
    {
        return true;
    }
}

/*--------------------------------------------------------------------------------*/
/*Program Output                                                                  */
/*   -Display number of corresponding days                                        */
/*--------------------------------------------------------------------------------*/
void getDays()
{
    ofs << endl;
    
    if (oddEven(month) == false)
    {
        if (month == 9 || month == 11)
        {
            ofs << showDays << (day = 30);
        }
        
        else
        {
            ofs << showDays << (day = 31);
        }
    }
    
    else
    {
        if (oddEven(month) == true)
        {
            if (month == 4 || month == 6)
            {
                ofs << showDays << (day = 30);
            }
            
            else
            {
                if (month == 2)
                {
                    if (year % 100 == 0)
                    {
                        if (year % 400 == 0)
                        {
                            ofs << showDays << (day = 29);
                        }
                    }
                    
                    else
                    {
                        if (year % 4 == 0)
                        {
                            ofs << showDays << (day = 29);
                        }
                        
                        else
                        {
                            ofs << showDays << (day = 28);
                        }
                    }
                }
                
                else
                {
                    ofs << showDays << (day = 31);
                }
            }
            
        }
    }
    
    ofs << endl;
}

/*--------------------------------------------------------------------------------*/
/*Parent Function                                                                 */
/*--------------------------------------------------------------------------------*/
int main()
{
	header();
	getTime();
	oddEven(month);
	getDays();
	footer();
	eofmsg();
	ofs.close();

	return 0;
}

/*--------------------------------------------------------------------------------*/
/*                                                                                */
/*    ______nd                                                                    */
/*   /_____/\       Name: David Bucio                                             */
/*   \::::_\/_      Date: November 1, 2016                                        */
/*    \:\/___/\     Program: Days in a Month                                      */
/*     \::___\/_    Summary: Debugging and Compiling Test                         */
/*      \:\____/\                                                                 */
/*       \_____\/                                                                 */
/*                                                                                */
/*--------------------------------------------------------------------------------*/
crestfallenDeus (5)
Thanks for your input! Although it makes my code look nice and clean, it does get to be a drag sometimes typing out all of the /*---*/ boxes. But my professor is a real stickler when it comes to how he likes to see our code formatted. Your suggestion sounds like a good compromise.
crestfallenDeus (5)
Anyways, what I really want to know is if there are any better ways of structuring my getDays() module? Without entering the realm of higher C++ knowledge, could it be condensed to just a few lines?
jlb (4973)
Instead of worrying about how to condense a function you would be better off worrying about writing good clean code. You really should stop using all of those global variables. Learn to create the variables where they're needed and how to properly pass values to and from your functions.

crestfallenDeus (5)
Okay, could you give me an example of how to do this? Any help is greatly appreciated. Also, I'm not sure if you're referring to all of my strings, but my professor is anal about not hard coding.
Last edited on
jlb (4973)
Your strings would possibly be okay, if they were const qualified. However if only one function actually uses these strings they probably should be defined in that function (still const qualified).


Okay, could you give me an example of how to do this?

You already have a small example of this, look at your oddEven() function. You're passing a value into that function. The problem here is that that variable is a global variable passing a global variable to a function as a parameter is a bad practice.

Next look at this snippet:
1
2
3
4
5
6
7
8
9
10
11
12
13
14
 
/*--------------------------------------------------------------------------------*/
/*Program Input                                                                   */
/*   -Prompt user to enter both a month and year                                  */
/*--------------------------------------------------------------------------------*/
void getTime()
{
    cout << askMonth;
    cin >> month;
    cout << endl;
    
    cout << askYear;
    cin >> year;
    cout << endl;
}


First the name of the function is misleading, no where in the function are you working with a "time".

Also because you're using those global strings you are really obscuring the program. IMO this should either be two different functions, one that returns the month and one that returns the year. Or you should be passing the variables by reference into this function. Something like:
1
2
3
4
5
6
7
8
9
10
11
12
13
14
 
void getMonthYear(int &month, int &year)
{
    cout << "Enter a numeric month: ";
    cin >> month;
    
    cout << "Enter a numeric year: ";
    cin >> year;

}
// In main().

   int month;
   int year;
   getMonthYear(month, year);



crestfallenDeus (5)
Duly noted. I guess I don't want to get the values of my variables mixed up. Off to study passing variables by reference. Any more advice is welcomed.
JLBorges (13770)
> Can my source code be condensed?

Yes.
It can also be extended (for instance, by adding validation of user input).

For example: 
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
 
#include <iostream>
#include <string> // *** EDIT: added

enum { Jan = 1, Feb, Mar, Apr, May, Jun, Jul, Aug, Sep, Oct, Nov, Dec };
static_assert( Dec == Jan + 11, "wrong number of months" ) ;
constexpr int calendar_start = 1583 ; // first full year of the gregorian calendar

// invariant: year > calendar_start
bool is_leap( int year )
{
   if( year%400 == 0 ) return true ;
   else return year%4 == 0 && year%100 != 0 ;
}

// invariant: month in [Jan,Dec],  year > calendar_start
int num_days_in_month( int month /* Jan == 1 */, int year )
{
    static const int ndays[] = { 0 /* not used */, 31, 28, 31, 30, 31, 30, 31, 31, 30, 31, 30, 31 } ;
    static_assert( sizeof(ndays) / sizeof( ndays[0] ) == Jan + 12, "wrong array size" ) ;

    if( month == Feb && is_leap(year) ) return 29 ;
    else return ndays[month] ;
}

// read in an integer in the range [minv,maxv]
int get_int( std::string prompt, int minv, int maxv )
{
    std::cout << prompt << " [" << minv << ',' << maxv << "]: " ;

    int value ;
    if( std::cin >> value )
    {
        if( value >= minv && value <= maxv ) return value ;
        else std::cout << "the value is out of range. " ;
    }
    else // non-numeric input
    {
        std::cout << "non-numeric input. " ;
        std::cin.clear() ; // clear the failed state
        std::cin.ignore( 1000000, '\n' ) ; // throw the bad input away
    }

    std::cout << "try again.\n" ;
    return get_int( prompt, minv, maxv ) ; // try again
}

int get_month() { return get_int( "month", Jan, Dec ) ; }

int get_year() { return get_int( "year", calendar_start, 999999 ) ; }

int main()
{
    const int month = get_month() ;
    const int year = get_year() ;
    std::cout << "days in the month: " << num_days_in_month( month, year ) << '\n' ;
}
Last edited on
ravenodin (4)
@JLBorges

You should include the <string> header when using std::string.
JLBorges (13770)
Yes. Thank you!
Topic archived. No new replies allowed.