patterncppMinor
It's almost Friday, right?
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.
```
/**
* @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
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
-
This output should be written to
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
-
You don't need to initialize the
-
Any member function, such as
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
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.