HiveBrain v1.2.0
Get Started
← Back to all entries
patterncppMinor

It's almost Friday, right?

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
almostfridayright

Problem

This is my final post for my CS1 saga. Mostly all I had to do was just "class-ify" my past project in this post. Here are the requirements:


Make all data members private and use constructors, accessors and
mutators. Enhancement: Build data verification into the member functions.

I'm mostly concerned with me doing OO programming correctly. I wasn't very good with it in my Java days way back yonder, and I'm not sure it's that good now either. Focus in this area would be appreciated.

classyDayOfWeek.cpp:

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

#include
#include
#include

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

/**
* The class associated with date values and calculating the day of the week
*/
class Date
{
public:
int dayOfWeek();
void getInput();
bool isLeapYear();
int getCenturyValue();
int getYearValue();
int getMonthValue();
private:
unsigned day = 0;
unsigned month = 0;
unsigned year = 0;
};

/**
* Makes sure data isn't malicious, and signals user to re-enter proper data if invalid
*/
unsigned getSanitizedNum()
{
unsigned 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::max(), '\n');
std::cout = 1582) && (!(year % 400) || (!(year % 4) && (year % 100)));
}

/**
* Finds the century, divides by 4, and returns the remainder.
*/
int Date::getCenturyValue()
{
return (2 * (3 - div(year / 100, 4).rem));
}

/**
* Computes a value based on the years since the beginning of the century.
*/
int Date::getYearValue()
{
int mod = year % 100;
return (mod + div(mod, 4).quot);
}

/**
* Returns a value based on the given hash tables

Solution

-
It seems unusual to have a member function that takes input, also considering that most of the work already done in the class is computational-based. Consider getting the input in main() and constructing a Date object from this, or overload operator>>.

-
This output should be written to std::cerr instead:

std::cout << "Invalid input.  Please enter a positive number: ";


This also makes it clear to the reader that this is not a standard output, but an erroneous one.

-
You have some magic numbers in isLearYear() and getCenturyValue(). Any of them can be in the respective functions as consts.

-
You don't need to initialize the private variables; that's for the constructor or initializer list.

-
Any member function, such as getCenturyValue(), that does not modify any data members should be const:

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


This not only makes the intent clearer, but also prevents any accidental modification of data members by giving a compiler error.

-
You could move your two free functions next to main() to avoid possible confusion with the member functions (at least while everything exists in one file).

Code Snippets

std::cout << "Invalid input.  Please enter a positive number: ";
int Date::getCenturyValue() const
{
    return (2 * (3 - div(year / 100, 4).rem));
}

Context

StackExchange Code Review Q#72294, answer score: 9

Revisions (0)

No revisions yet.