29
\$\begingroup\$

This is the first program that I have written in my C++ saga that I actually think is useful. The description for this assignment is kinda long and mundane though:

Write a program that inputs a date (for example: July 4, 2008) and outputs the day of the week that corresponds to that date. The following algorithm is from Wikipedia. The implementation will require several functions.

  • bool isLeapYear(int year);

    This function should return true if year is a leap year and false if it is not. Here is the pseudo code to determine a leap year:

    leap_year = (year divisible by 400) or (year divisible by 4 and not divisible by 100)
    
  • int getCenturyValue(int year);

    This function should take the first two digits of the year (that is, the century), divide by \$ 4 \$, and save the remainder. Subtract the remainder from \$ 3 \$ and return this value multiplied by \$ 2 \$. For example, the year \$ 2008 \$ becomes \$ \dfrac{20}{4} = 5 \$ with a remainder of \$ 0 \$. \$ 3 - 0 = 3 \$. Return \$ 3 \cdot 2 = 6 \$.

  • int getYearValue(int year);

    This functions computes a value based on the years since the beginning of the century. First, extract the last two digits of the year. For example, \$ 08 \$ is extracted for \$ 2008 \$. Next, factor in leap years. Divide the value from the previous step by \$ 4 \$ and discard the remainder. Add the two results together and return this value. For example, from \$ 2008 \$ we extract \$ 08 \$. Then \$ \dfrac{8}{4} = 2 \$ with a remainder of \$ 0 \$. Return \$ 2 + 8 = 10 \$.

  • int getMonthValue(int month, int year);

    This function should return a value based on the table below and will require invoking the isLeapYear() function.

    $$ \newcommand\T{\Rule{0pt}{1em}{.5em}} \begin{array}{|c|c|} \hline \textbf{Month} & \textbf{Return Value} \\\hline \text{January} \T & 0 \left(6 \text{ if year is a leap year} \right) \\\hline \text{February} \T & 3 \left(2 \text{ if year is a leap year} \right) \\\hline \text{March} \T & 3 \\\hline \text{April} \T & 6 \\\hline \text{May} \T & 1 \\\hline \text{June} \T & 4 \\\hline \text{July} \T & 6 \\\hline \text{August} \T & 2 \\\hline \text{September} \T & 5 \\\hline \text{October} \T & 0 \\\hline \text{November} \T & 3 \\\hline \text{December} \T & 5 \\\hline \end{array} $$

  • Finally, to compute the day of the week, compute the sum of the date's day plus the values returned by getMonthValue, getYearValue, and getCenturyValue. Divide the sum by \$ 7 \$ and compute the remainder. A remainder of \$ 0 \$ corresponds to Sunday, \$ 1 \$ corresponds to Monday, etc., up to \$ 6 \$, which corresponds to Saturday. For example, the date July 4, 2008 should be computed as (day of month) + (getMonthValue) + (getYearValue) + (getCenturyValue) = \$ 4 + 6 + 10 + 6 = 26 \$. \$ \dfrac{26}{7} = 3 \$ with a remainder of \$ 5 \$. The fifth day of the week corresponds to Friday.

Your program should allow the user to enter any date and output the corresponding day of the week in English.

This program should include a void function named getInput that prompts the user for the date and returns the month, day, and year using pass-by-reference parameters. You may choose to have the user enter the date's month as either a number (1-12) or a month name.

dayOfWeek.cpp:

/**
 * @file dayOfWeek.cpp
 * @brief Computes the day of the week given a certain date
 * @author syb0rg
 * @date 10/23/14
 */

#include <cctype>
#include <iostream>
#include <limits>

enum Months {None, January, February, March, April, May, June, July, August, September, October, November, December};
std::string *weekDays = new std::string[7] {"Sunday", "Monday", "Tuesday", "Wednesday", "Thursday", "Friday", "Saturday"};

/**
 * Makes sure data isn't malicious, and signals user to re-enter proper data if invalid
 */
size_t getSanitizedNum()
{
    size_t input = 0;
    while(!(std::cin >> input))
    {
        // clear the error flag that was set so that future I/O operations will work correctly
        std::cin.clear();
        // skips to the next newline
        std::cin.ignore(std::numeric_limits<std::streamsize>::max(), '\n');
        std::cout << "Invalid input.  Please enter a positive number: ";
    }
    return input;
}

/**
 * Safetly grabs and returns a lowercase version of the character (if the lowercase exists)
 */
char32_t getSanitizedChar()
{
    // absorb newline character (if existant) from previous input
    if('\n' == std::cin.peek()) std::cin.ignore();
    return std::tolower(std::cin.get());
}

bool isLeapYear(const int year)
{
    if (0 == (year % 400) || (0 == (year % 4) && (year % 100))) return true;
    else return false;
}

int getCenturyValue(const size_t year)
{
    return (2 * (3 - div(year / 100, 4).rem));
}

int getYearValue(const size_t year)
{
    int mod = year % 100;
    return (mod + div(mod, 4).quot);
}

int getMonthValue(const size_t month, const size_t year)
{
    switch (month) {
        case January:
            if (isLeapYear(year)) return 6;
        case October:
            return 0;
        case May:
            return 1;
        case August:
            return 2;
        case February:
            if (isLeapYear(year)) return 2;
        case March:
        case November:
            return 3;
        case June:
            return 4;
        case September:
        case December:
            return 5;
        case April:
        case July:
            return 6;
        default:
            return -1;
    }
}

int dayOfWeek(const size_t month, const size_t day, const size_t year)
{
    return div(day + getMonthValue(month, year) + getYearValue(year) + getCenturyValue(year), 7).rem;
}

void getInput(size_t &month, size_t &day, size_t &year)
{
    std::cout << "Enter the month (1-12): ";
    month = getSanitizedNum();

    std::cout << "Enter the day (1-31): ";
    day = getSanitizedNum();

    std::cout << "Enter the year: ";
    year = getSanitizedNum();
}

int main()
{
    size_t month = 0;
    size_t day = 0;
    size_t year = 0;
    do
    {
        getInput(month, day, year);
        std::cout << "The day of the week is " << weekDays[dayOfWeek(month, day, year)] << std::endl;

        std::cout << "Run the program again (y/N): ";  // signify n as default with capital letter
    } while ('y' == getSanitizedChar());
}
\$\endgroup\$
5
  • 6
    \$\begingroup\$ In practice, you never want to write core feature of time libraries yourself. They are maddeningly complex to get correct, and what's correct today might not be correct tomorrow. If this wasn't a homework assignment, I would suggest learning to use Boost.Date_Time. \$\endgroup\$
    – Chuu
    Oct 23, 2014 at 14:45
  • \$\begingroup\$ @Chuu very good point. I was amazed by this example the first time I saw it. Never did I think telling time could be so complicated as to actually account for situations like that. I'm glad those time libraries take care of that for me! \$\endgroup\$ Oct 23, 2014 at 18:26
  • 2
    \$\begingroup\$ No leap year before 1582. Assuming Gregorian calendar. \$\endgroup\$ Oct 23, 2014 at 23:40
  • 1
    \$\begingroup\$ "This function should take the first two digits of the year (that is, the century)" Not necessarily true (In fact, for most years it's wrong). \$\endgroup\$ Oct 24, 2014 at 7:22
  • 4
    \$\begingroup\$ There is madness in this sort of code. \$\endgroup\$
    – nwp
    Dec 8, 2014 at 8:10

4 Answers 4

34
\$\begingroup\$

This is improper in a few ways:

std::string *weekDays = new std::string[7] {"Sunday", "Monday", "Tuesday", "Wednesday", "Thursday", "Friday", "Saturday"};

Not only is this a global variable (no const), but it's being dynamically allocated without being deallocated with delete somewhere. Plus, it can be deallocated anywhere, and possibly more than once if you're not careful. Either way, since std::string already does its own memory management, just leave out new.

Assuming no C++11 (no access to std::array), you should end up with this:

const std::string weekDays[] = {"Sunday", "Monday", "Tuesday", "Wednesday", "Thursday", "Friday", "Saturday"};

In C++, it's also best to leave the [] empty when initializing a C array. The compiler will determine the correct size by itself, so you don't need to worry about keeping it up-to-date yourself.

\$\endgroup\$
22
\$\begingroup\$

I'll just focus on getMonthValue():

int getMonthValue(const size_t month, const size_t year)
{
    switch (month) {
        case January:
            if (isLeapYear(year)) return 6;
        case October:
            return 0;
        case May:
            return 1;
        …

The month and year parameters aren't sizes. Don't use size_t when you just mean unsigned int. However, you also had bool isLeapYear(const int year) — why the inconsistency in the type of year? Furthermore, you have a perfectly good Months enum defined. Why not rename it using the singular and use that? You're implicitly comparing size_t and Months in the switch block anyway.

Instead of the switch block, I'd use a lookup table. Actually, two lookup tables: one for leap years and one for non-leap years. A data-driven design would have less code.

int getMonthValue(int month, int year) {
    const static int NON_LEAP_VALS[] = { 0xDEAD, 0, 3, 3, 6, 1, 4, 6, 2, 5, 0, 3, 5 },
                         LEAP_VALS[] = { 0xDEAD, 6, 2, 3, 6, 1, 4, 6, 2, 5, 0, 3, 5 };
    return (isLeapYear(year)) ? LEAP_VALS[month] : NON_LEAP_VALS[month];
}
\$\endgroup\$
1
  • \$\begingroup\$ Is the use of dummy 0xDEAD entry instead of using [month+1] index an instance of data-driven design? \$\endgroup\$
    – Ruslan
    Aug 28, 2017 at 16:22
15
\$\begingroup\$
bool isLeapYear(const int year)
{
    if (0 == (year % 400) || (0 == (year % 4) && (year % 100))) return true;
    else return false;
}

This is nitpicky, but this is a bit unreadable to me. The inconsistent use of parentheses makes me think hard about order of evaluation.

Also, you use 0 == to express a false value, but you don't use 0 != to express a true value, which I also find inconsistent.

Also, the if/else is a bit redundant.

I would rewrite this as:

bool isLeapYear(const int year)
{
    return (year % 400 == 0) || ((year % 4 == 0) && (year % 100 != 0));
}

or if not using == 0:

bool isLeapYear(const int year)
{
    return !(year % 400) || (!(year % 4) && (year % 100));
}
\$\endgroup\$
5
  • 1
    \$\begingroup\$ I also think the year % 100 logic is incorrect - it is a leap year if it is divisible by 4 and not 100. \$\endgroup\$ Oct 23, 2014 at 17:21
  • 3
    \$\begingroup\$ @GalacticCowboy year % 100 is the same as writing year % 100 != 0, as any non-zero result is equivalent to true. \$\endgroup\$
    – Rotem
    Oct 23, 2014 at 17:23
  • \$\begingroup\$ The way you have it, yes. I think his original logic is flawed, though. \$\endgroup\$ Oct 23, 2014 at 17:38
  • 4
    \$\begingroup\$ @GalacticCowboy I don't see why. == has higher precedence than &&, the 0 == (year % 4) will be evaluated before (year % 100). Am I missing something? \$\endgroup\$
    – Rotem
    Oct 23, 2014 at 18:04
  • 1
    \$\begingroup\$ While what you have is synactically correct, it's more human readable to use: return ((year % 400) == 0) || (((year % 4) == 0) && ((year % 100) != 0)); \$\endgroup\$
    – R Sahu
    Dec 8, 2014 at 3:07
3
\$\begingroup\$

I got the code and gave an improvement based on the comments here and I changed some things mainly the function isLeapYear because it is not necessary to create a function inside the iostream already has the __isleap is running 100%: https://pastebin.com/7xsM2CSF

#include <limits> //numeric_limits
#include <iostream>
//https://codereview.stackexchange.com/questions/67636/is-it-friday-yet
//dayOfWeek.cpp

enum Months { January=1, February, March, April, May, June, July, August, September, October, November, December};

const std::string weekDays[7]={"Sunday", "Monday", "Tuesday", "Wednesday", "Thursday", "Friday", "Saturday"};

/**
 * Makes sure data isn't malicious, and signals user to re-enter proper data if invalid
 * getSanitizedNum
 */
int getInt()
{
    int input = 0;
    while(!(std::cin>>input))
    {
        // clear the error flag that was set so that future I/O operations will work correctly
        std::cin.clear();
        // skips to the next newline
        std::cin.ignore(std::numeric_limits<std::streamsize>::max(), '\n');
        std::cout << "\n\tInvalid input.\n\tPlease enter a positive number: ";
    }
    return input;
}

/**
 * Safetly grabs and returns a lowercase version of the character (if the lowercase exists)
 * getSanitizedChar
 */
char32_t getChar()
{
    // absorb newline character (if existant) from previous input
    if('\n' == std::cin.peek()) std::cin.ignore();
    return std::tolower(std::cin.get());
}

int getCenturyValue(int year)
{
    return (2 * (3 - div(year / 100, 4).rem));
}

int getYearValue(int year)
{
    int mod = year % 100;
    return (mod + div(mod, 4).quot);
}

int getMonthValue(int month, int year) 
{
    const static int NON_LEAP_VALS[] = { 0xDEAD, 0, 3, 3, 6, 1, 4, 6, 2, 5, 0, 3, 5 },
                         LEAP_VALS[] = { 0xDEAD, 6, 2, 3, 6, 1, 4, 6, 2, 5, 0, 3, 5 };

  return (__isleap(year) ? LEAP_VALS[month] : NON_LEAP_VALS[month]);
}
/*
int getMonthValue(int month, int year)
{
    switch (month) 
    {
        case January:
            if (__isleap(year)) return 6;
        case October:
            return 0;
        case May:
            return 1;
        case August:
            return 2;
        case February:
            if (__isleap(year)) return 2;
        case March:
        case November:
            return 3;
        case June:
            return 4;
        case September:
        case December:
            return 5;
        case April:
        case July:
            return 6;
        default:
            return -1;
    }
}
*/
int dayOfWeek(int day, int month, int year)
{
    return div(day + getMonthValue(month, year) + getYearValue(year) + getCenturyValue(year), 7).rem;
}

void getInput(int &day, int &month, int &year)
{
 int meses[12] = {31,28,31,30,31,30,31,31,30,31,30,31};
 do{
    do{
       std::cout << "\n\tEnter the day (1-31): ";
       day = getInt();
      }while(day < 1 || day > 31);

    do{  
       std::cout << "\n\tEnter the month (1-12): ";
       month = getInt();
      }while(month < 1 || month > 12);

    do{  
       std::cout << "\n\tEnter the year: ";
       year = getInt();
      }while(year < 1100);

      if(__isleap(year))meses[1] = 29;

      if(day > meses[month-1])std::cout<<"\n\tO mes "<<month<<" do ano de "<<year<<" nao tem "<<day<<" dias!!!\n\n";

  }while(day > meses[month-1]);
}

int main()
{
 int day = 0;
 int month = 0;
 int year = 0;

 do{
    getInput(day, month, year);
    std::cout << "\n\tThe day of the week is " << weekDays[dayOfWeek(day, month, year)] << std::endl;

    std::cout << "\n\tRun the program again (y/N): ";  // signify n as default with capital letter

   }while('y' == getChar());

  return 0;
}
\$\endgroup\$
2
  • \$\begingroup\$ It will be useful to explain the change you did to isLeapYear() and why that is better \$\endgroup\$ Dec 23, 2017 at 13:08
  • \$\begingroup\$ not to reinvent the wheel if there is a specific function in the language for this. \$\endgroup\$
    – user148779
    Dec 23, 2017 at 14:03

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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