6
\$\begingroup\$

This is one of my first C++ training project where I created the console application which displays the calendar of any given month.

I observed how offset of days (empty places on the calendar) change year by year and build the empirical formula on it. For the starting point (the fixed year) I have chosen 2008 only because I worked on this ten years ago.

Now I changed and fixed several things in that old code. However, I am sure there are much more which could be done better or improved.

//Comment next line if you do not use MS-DOS compilers
#define MSDOS    

#include <iostream>
#include <string.h>

#ifdef MSDOS
#include <conio.h>
#endif
using namespace std;

void Calendar(int year, int month);
int GetUserInput(const char * name, int from = 0, int to = 0);

int main()
{
    int year, month;
    char control = 0;
    char menu[] = "< P > - Previous,  < N > - Next\n< D > - New date,  < E > - Exit\n";

    year = GetUserInput("Year");
    month = GetUserInput("Month", 1, 12);

    Calendar(year, month);
    cout << menu;

    do
    {
        bool Proceed = false;

#ifdef MSDOS
        Proceed = _kbhit();
#else
        Proceed = true;
#endif

        if (Proceed)
        {
#ifdef MSDOS
            control = _getch();
#else
            cin.get(control);
            cin.get();
#endif
            system("cls");

            switch (control)
            {
            case -32:
                continue;
                break;
            case 'P':
            case 'p':
            case 72:    // Up arrow
            case 75:    // Left arrow
                month--;
                if (month < 1)
                {
                    month += 12;
                    year -= 1;
                }
                break;
            case 'N':
            case 'n':
            case 13:    // Enter
            case 77:    // Right arrow
                month++;
                if (month > 12)
                {
                    month -= 12;
                    year += 1;
                }
                break;
            case 'D':
            case 'd':
                year = GetUserInput("Year");
                month = GetUserInput("Month", 1, 12);
                break;
            case 'E':
            case 'e':
                cout << "\n\tBye!\n";
                break;
            default:
                cout << "\n\tWrong input!\n\n";
                cout << menu;
                continue;
                break;
            }

            Calendar(year, month);
            cout << menu;
        }
    } while (control != 'e' && control != 'E');

    return 0;
}

int GetUserInput(const char * name, int from, int to)
{
    bool succeed = false;
    int value = 0;
    char buffer[10];
    do
    {
        cout << "Enter " << name << ": ";
        cin.getline(buffer, 10);
        value = atoi(buffer);

        if (from == to || value >= from && value <= to)
            succeed = true;
    } while (!succeed);

    return value;
}

void Calendar(int year, int month)
{
    char monthName[][10]
    {
        "January", "February", "March", "April", "May", "June",
        "July", "August", "September", "October", "November", "December"
    };

    int numberOfDaysInMonth, shift, empty;
    int daysInMonth[]{ 31, 28, 31, 30, 31, 30, 31, 31, 30, 31, 30, 31 };
    numberOfDaysInMonth = (month == 2 && year % 4 == 0) ? 29 : daysInMonth[month - 1];

    const int fixedYear = 2008;
    const int fixedEmpty[]{ 1, 4, 5, 1, 3, 6, 1, 4, 0, 2, 5, 0 };

    int yearDiff = year - fixedYear;

    if (yearDiff > 0)
    {
        shift = yearDiff + yearDiff / 4;
        if (month <= 2 && year % 4 != 0)
            shift++;
    }
    else
    {
        shift = yearDiff - yearDiff / 4;
        if (month > 2 && year % 4 != 0)
            shift--;
    }

    empty = fixedEmpty[month - 1] + shift;

    empty = empty % 7;
    if (empty < 0)
        empty += 7;

    cout << " ................... " << monthName[month - 1] << " " << year << " ";
    int dots = 23 - strlen(monthName[month - 1]);
    for (int i = 0; i < dots; i++)
        cout << ".";
    cout << "\n Mo\tTu\tWe\tTh\tFr\tSa\tSu\t\n ";

    for (int e = 0; e < empty; e++)
        cout << "\t";
    for (int d = 1; d <= numberOfDaysInMonth; d++)
        cout << d << ((empty + d) % 7 == 0 ? "\n " : "\t");
    cout << "\n\n";
}

I wonder if there is any conventional way to determine the weekday for the start a month based on only month and year.

It would be great if someone could show me what is common practice to manage different libraries for different compilers (like the <conio.h> in my case).

\$\endgroup\$
8
\$\begingroup\$

If you have a C++ standards-compliant compiler, then you should not need to use <conio.h> under Windows (_kbhit and _getch). std::cin will work. I know there are still old compilers floating around, but there are various modern C++ compilers available for free (GCC, Clang and MSVC), I don't think there is any excuse to code for non-standards compliant compilers if you're coding as a hobby.

The thing that is not portable is system("cls");. This will cause an error under any non-Windows system that does not define a cls shell command. This is the part that should be guarded by a #ifdef MSDOS.

However, if you are writing code that must support a large collection of compilers, and need to code around peculiarities of individual compilers, use preprocessor macros specifically defined by those compilers. The "Pre-defined Compiler Macros" project is a great resource to find these macros (and also OS-specific ones). Using those you don't need to change the value of MSDOS in the source file when you compile.

Don't use #include <string.h>, which is a C header file, use #include <cstring>, which is a C++ header file in the C++ standard library.

Don't use using namespace std.

case -32:
   continue;
   break;

The break is superfluous, no statements after continue will be executed.

month--;
if (month < 1)
{
   month += 12;
   year -= 1;
}

Here I would not add 12 to month, but directly assign 12. Not that the difference is significant in any way, but I think it's a little cleaner.

Also, it is considered good practice to prefer the pre-increment operator over the post-increment operator, that is --month instead of month--.

do
{
   // ...
   switch (control)
   {
   // ...
   case 'E':
   case 'e':
      cout << "\n\tBye!\n";
      break;
   }
} while (control != 'e' && control != 'E');
return 0;

You can simplify this control loop a bit: under case 'e', directly return (or exit). The do{}while() loop becomes a while(true){} loop.

Note that return 0; is not necessary at the end of main, as it is implied.

The above is true also for the control loop in GetUserInput. Instead of setting succeed to true, and testing for succeed in the loop, make it an infinite loop and return value inside the loop if the value is OK.

cin.getline(buffer, 10);
value = atoi(buffer);

This looks like it was translated from C. Why not this?

std::cin >> value;

Your test for leap days is not correct:

(month == 2 && year % 4 == 0)

Years divisible by 100 are not leap years, unless they are divisible by 400.

char monthName[][10] = { /* 12 month names */ };

looks weird. Because there's a 10 there, but I know there are 12 months. It took me a second to realize you are allocating 10 characters to store the names. Instead, store the pointers to the constant char arrays:

char const* monthName[] = { /* 12 month names */ };

I don't know how the compiler optimizes this code, but in principle char monthName[][10] is an array with 12*10 chars, copied over every time your function is called from the given constant strings (though likely the compiler will optimize this out in some way).

Finally, this being C++, I would recommend that you create a class for the date. The calculations and the printing of the calendar would be member functions for this class. Advantages are some simplifications in the code in main (more stuff would be pushed into member functions), and reusability (it would be easier to use your date calculations in a new program). For example, you would define operations like these:

date.read();       // prompts to std::cout and reads from std::cin
std::cout << date;
++date;
\$\endgroup\$
  • 1
    \$\begingroup\$ I wouldn't recommend exit() in C++ in any situation. Also, getline() followed by atoi() (or something similar) is actually the correct thing to do. cin >> value will leave trailing characters (including the newline), which will screw up later calls to cin.get(). Also, doing that adds the complexity of error handling with cin. \$\endgroup\$ – indi Jun 24 '18 at 6:15
  • 1
    \$\begingroup\$ Also, that "predefined macros" site is a terrible thing to recommend: it not only encourages bad practices (and in some cases, straight-up UB), there are better ways to handle most anything you might need its advice for. In this case, the best advice for how to use system("cls") is simply: don't. Wrapping it in a macro is not a solution. \$\endgroup\$ – indi Jun 24 '18 at 6:27
  • \$\begingroup\$ @indi: Thanks for your feedback. I guess I don’t have a lot of experience with cin. — I have found the compiler version macros from that site to be really helpful in working around bugs (standard non-compliances) in various versions of MSVC. Sure, ideally you write code for one compiler and one platform, but we don’t all have that luxury. I don’t see how testing for a compiler version using the preprocessor is UB. \$\endgroup\$ – Cris Luengo Jun 24 '18 at 14:38
6
\$\begingroup\$

I found the basic model of how the program works more annoying than useful. I'd expect typing just "cal" to pick a reasonable default, such as displaying the calendar for the current month (and possibly the previous and next months).

I'd use the standard library for most of the calculations unless you have a specific reason to do otherwise (e.g., wanting to be able to display ancient dates, such as the calendar for the month Julius Caesar was born, or something like that (but that's going to take a fair amount of work to do correctly--you have to keep track of when people switched from the Julian to Gregorian calendar, and things like that).

For most purposes, it's fine to just support reasonably-current and near-future dates, in which case the standard library is adequate.

So, as a starting point, consider some code to display a calendar for the current month:

#include <ctime>
#include <iostream>
#include <iomanip>
#include <string>

void show_cal(tm target) {
    target.tm_mday = 1;
    mktime(&target);

    char const *months[] = { "January", "February", "March", "April", "May", "June",
                        "July", "August", "September", "October", "November", "December" };

    std::cout << months[target.tm_mon] << " " << target.tm_year + 1900 << "\n";

    int dow = target.tm_wday;
    std::cout << "Sun Mon Tue Wed Thu Fri Sat\n";

    std::cout << std::string(4 * dow, ' ');
    int month = target.tm_mon;

    for (int i = 1; target.tm_mon == month; target.tm_mday = ++i, mktime(&target)) {
        std::cout << std::setw(3) << std::setprecision(3) << i << ' ';
        if (((i+dow) % 7) == 0)
            std::cout << "\n";
    }
}

int main() {
    std::time_t now = time(NULL);

    show_cal(*gmtime(&now));
}

This is certainly open to improvement, such as using the locale to display the names for the month and days of the week properly for the user's locale. On the other hand, it's at least a pretty straightforward way of showing a calendar for a month reasonably well (within the limits of the standard library).

From there supporting other months/years is mostly a matter of parsing command line arguments to figure out what the user wants (which can vary from quite simple (support only one format, such as cal month year) to quite elaborate (e.g., allow selecting an entire year or a single month/year or a single month of the current year, or ...)

If you really do want to support ancient dates and such, you might want to take a look at the date/time routines from the old C Snippets archive:

https://github.com/vonj/snippets.org/blob/master/cal.c https://github.com/vonj/snippets.org/blob/master/jdn_l.c

\$\endgroup\$
  • \$\begingroup\$ Doesn’t the standard library already offer nationalized date formatting? You should take your own advice and not list the longname months yourself. strftime or the new chrono::format. \$\endgroup\$ – JDługosz Jun 25 '18 at 6:42
  • \$\begingroup\$ @JDługosz: I wonder if that's why I said: "This is certainly open to improvement, such as using the locale to display the names for the month and days of the week properly for the user's locale"? Getting the month would be easy. The day names isn't terribly difficult, but the code is kind of confusing and distracting--you basically start from the first of the month, increment until you get to a Sunday, then increment through and print out day names for a week. Not difficult, but confusing nonetheless. \$\endgroup\$ – Jerry Coffin Jun 25 '18 at 7:01

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.