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

C++ calendar printer

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

Problem

I was at CppCon last week and watched a clever talk about using ranges and calendars. This is just me trying to play around with the new technology.

Step 1: dump out a calendar.

Usage:

cal


Output:

January
Sun Mon Tue Wed Thu Fri Sat
01 02 03 04
05 06 07 08 09 10 11
...etc


`#include
#include
#include

enum WeekDay {Sun, Mon, Tue, Wed, Thu, Fri, Sat};

class Calander
{
/*
* Line 0:
* Line 1: Sun Mon Tue Wed Thu Fri Sat
* Line 2: Week 1 (May need leading space)
* Line 3: Week 2
* Line 4: Week 3
* Line 5: Week 5 or (blank)
* Line 6: Week 6 or (blank)
* Line 7: (blank)
*/
class WeekLine
{
static int getDaysInMonth(int month, bool isLeap)
{
static constexpr int normal[] = {31, 28, 31, 30, 31, 30, 31, 31, 30, 31, 30, 31};
static constexpr int leap[] = {31, 29, 31, 30, 31, 30, 31, 31, 30, 31, 30, 31};

return isLeap ? leap[month] : normal[month];
}
static bool isLeapYear(int year)
{
return (year % 400 == 0) || ((year % 4 == 0) && (year % 100 != 0));
}

static int getOffsetOfFirstDay(int year, int month)
{
struct ::tm timeS;
timeS.tm_sec = 0;
timeS.tm_min = 0;
timeS.tm_hour = 0;
timeS.tm_mday = 1;
timeS.tm_mon = month;
timeS.tm_year = year - 1900;
timeS.tm_isdst = 0;
timeS.tm_zone = nullptr;
timeS.tm_gmtoff = 0;
timegm(&timeS);

// TODO:
// Other week start days apart from Sunday.
return timeS.tm_wday; // day of week (Sunday = 0)
}

int year;
bool isLeap;
int month;
int dayOfMonth;
WeekDay firstDayOfWeek;
int offset;
int l

Solution

-
One small change

class WeekLine
    : public std::iterator


will allow it to be used like this

std::copy(cal.begin(), cal.end(), 
    std::ostream_iterator(std::cout));


Somewhat useless, but at least it'll be usable with the standard library.

Speaking of which, is there a reason why your WeekLine is private? At least do something like this:

using iterator = WeekLine;


That way I can replace decltype(cal.begin()) with Calander::iterator.

-
You should use ` instead of .

-
std::atoi comes from . Don't omit headers since it may fail to compile on another implementation.

-
Why not use an
enum class for WeekDay? You never use it, for numerical values or anything else, so there's no point in injecting Sun, Mon, etc. into the global namespace. Make it private like the other classes.

-
In one case of declaring an array, you use
constexpr, in another you use const. constexpr is implicitly const`, so for consistency, choose one or the other. There also probably isn't any difference semantically in these situations.

-
I noticed you using C++14 digit separators. In this case, I recommend against it since 1) support may not be available on all compilers, 2) it's confusing to read even for people who know what they are. At least format your function:

bool operator!=(WeekLine const& rhs)   
{
   return year * 10'000 
     + month * 100 
     + dayOfMonth != (rhs.year * 10'000) 
     + (rhs.month * 100) 
     + rhs.dayOfMonth;
}


-
Here's a more efficient way to check for a leap year:

if year is not divisible by 4 then not leap year
else if year is not divisible by 100 then leap year
else if year is divisible by 400 then leap year
else not leap year




This "most-efficient" pseudo-code simply changes the order of tests so the division by 4 takes place first, followed by the less-frequently occurring tests. Because "year" does not divide by four 75-percent of the time, the algorithm ends after only one test in three out of four cases.

And the actual code:

if ((year & 3) == 0 && ((year % 25) != 0 || (year & 15) == 0))
{
    /* leap year */
}


A comprehensive explanation/rationale can be found in the linked page.

Code Snippets

class WeekLine
    : public std::iterator<std::forward_iterator_tag, void, void, void, void>
std::copy(cal.begin(), cal.end(), 
    std::ostream_iterator<decltype(cal.begin())>(std::cout));
using iterator = WeekLine;
bool operator!=(WeekLine const& rhs)   
{
   return year * 10'000 
     + month * 100 
     + dayOfMonth != (rhs.year * 10'000) 
     + (rhs.month * 100) 
     + rhs.dayOfMonth;
}
if year is not divisible by 4 then not leap year
else if year is not divisible by 100 then leap year
else if year is divisible by 400 then leap year
else not leap year

Context

StackExchange Code Review Q#105960, answer score: 4

Revisions (0)

No revisions yet.