3
\$\begingroup\$

Recently I wrote a program that was required to handle year and month data and so I wrote this class to encapsulate that handling. What I needed was a way to initialize the Yearmonth object based on the current local time, and allow a simple method of calculating future Yearmonth values based on a duration in months. This sample code illustrates how I use it:

ymtest.cpp

#include <iostream>
#include "Yearmonth.h"
int main()
{
 YM::Yearmonth ym; // today
 std::cout << ym << '\n';
 ym += 2; // 2 months from now
 std::cout << ym << '\n';
 ym += 14; // test year increment
 std::cout << ym << '\n';
}

Yearmonth.h

#ifndef YEARMONTH_H
#define YEARMONTH_H
#include <iostream>
namespace YM {
class Yearmonth
{
public:
 // construct with today's year and month
 Yearmonth();
 // construct with year, month (1=Jan, 12=Dec)
 Yearmonth(unsigned ayear, unsigned amonth);
 // increment by given number of months
 Yearmonth &operator+=(const unsigned mon);
 // return year
 unsigned year() const;
 // return month (1=Jan, 12=Dec)
 unsigned month() const;
 // prints to ostream. E.g. 2014 Dec ==> "201412"
 friend std::ostream& operator<<(std::ostream &out, const Yearmonth &ym);
private:
 unsigned myyear;
 unsigned mymonth;
};
}
#endif //YEARMONTH_H

Yearmonth.cpp

#include <ctime>
#include "Yearmonth.h"
namespace YM {
Yearmonth::Yearmonth()
{
 time_t tt;
 time(&tt);
 tm *t = localtime(&tt);
 myyear = t->tm_year + 1900;
 mymonth = t->tm_mon;
}
Yearmonth::Yearmonth(unsigned ayear, unsigned amonth)
 : myyear(ayear), mymonth(amonth-1)
{
 myyear += mymonth/12;
 mymonth %= 12;
}
unsigned Yearmonth::year() const
{
 return myyear;
}
unsigned Yearmonth::month() const
{
 return mymonth+1;
}
Yearmonth &Yearmonth::operator+=(const unsigned mon)
{
 mymonth += mon;
 myyear += mymonth/12;
 mymonth %= 12;
 return *this;
}
std::ostream& operator<<(std::ostream &out, const Yearmonth &ym)
{
 return out << ym.myyear * 100 + ym.mymonth+1;
}
}

The class seems sufficient for my needs and everything works. Have I missed anything important?

200_success
145k22 gold badges190 silver badges478 bronze badges
asked Apr 22, 2015 at 15:34
\$\endgroup\$

2 Answers 2

4
\$\begingroup\$

I only have a few small remarks to make:

  • In your header file, don't include <iostream>: include <iosfwd> instead which contains the forward declarations for every type in <iostream>.

  • Also, you only use std::ostream in your source file, so you could simply include <ostream> there.

  • Several times, you compute mymonth / 12 and mymonth % 12. If your class is designed to be used intensively, you could consider using std::div(mymonth, 12) which will compute both values at once and may therefore be slightly faster (if you really need it).

  • You may want to prefix time_t, time, tm and localtime with std::. Them coming from the C standard library doesn't prevent you to use std::.

  • Using a const unsigned parameter seems pretty useless. I don't have strong opinions on the const on value parameters, but you could safely drop it since it adds little value.

answered Apr 22, 2015 at 15:49
\$\endgroup\$
3
\$\begingroup\$

I would consider your class to be good, but the most complex part of your code is the math of adding months to it. You are already doing modulo-arithmetic in the default constructor, so I would just make your underlying unit a single unsigned value.... which is a literal year * 12 + month value. This makes any arithmetic easy, and the presentation of the values is easy too. It also saves an unsigned value of space in memory.... note that there is no year 0.... and also that it is now limited to the year 5K or so, on 16-bit-int machines.

The cost of the modulo will be low on the get-methods, and I doubt it would be a significant part of any performance issues.

In other words, I would define the class as having the single unsigned myyearmonth; and then implement the class as:

Yearmonth::Yearmonth()
{
 time_t tt;
 time(&tt);
 tm *t = localtime(&tt);
 unsigned year = t->tm_year + 1900;
 unsigned month = t->tm_mon;
 myyearmonth = year * 12 + month;
}
Yearmonth::Yearmonth(unsigned ayear, unsigned amonth)
{
 myyearmonth = ayear * 12 + amonth - 1;
}
unsigned Yearmonth::year() const
{
 return myyearmonth / 12;
}
unsigned Yearmonth::month() const
{
 return (myyearmonth % 12) + 1;
}
Yearmonth &Yearmonth::operator+=(const unsigned mon)
{
 myyearmonth += mon;
 return *this;
}
std::ostream& operator<<(std::ostream &out, const Yearmonth &ym)
{
 return out << ym.year() * 100 + ym.month();
}
answered Apr 22, 2015 at 15:53
\$\endgroup\$
2
  • \$\begingroup\$ amonth in the constructor is 0-based, but Yearmonth::month() returns 1-based values? \$\endgroup\$ Commented Apr 22, 2015 at 17:47
  • \$\begingroup\$ @200_success - yeah the amonth would be input as 1-based, agreed, so the constructor should reduce that to a 0-based month, which I have now edited it to do. \$\endgroup\$ Commented Apr 22, 2015 at 17:51

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

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

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.