[boost] Period calculations in Boost Date-Time

From: Chris Trengove (trengove_at_[hidden])
Date: 2003-08-18 22:25:37

I have some concerns about the implementation of the "period" concept, as
given in period.hpp. First of all, the documentation (for, say, date_period)
talks of the constructor

date_period(date begin,date last)

as creating a period as [begin,last). But really what is meant here is that
the "last" parameter is actually going to become the "end" of the period.
This is clear from the implementation in period.hpp, which is

template<class point_rep, class duration_rep>
  inline
  period<point_rep,duration_rep>::period(point_rep begin, point_rep end) :
    begin_(begin),
    last_(end - duration_rep::unit())
  {}

In the other words, the last_ member is initialised by backing up one unit
of time from the "end".

I think there are two errors in the implementation of the period class,
perhaps caused by this confusion between "last" and "end".

1) is_null() is implemented as

    return last_ <= begin;

which means that a period of length 1 is judged as being "null". The correct
implementation should be

    return end() <= begin;
or
    return last < begin;

2) operator<() is implemented as

    return (last_ <= rhs.begin_);

and this should be

    return (end() <= rhs.begin_);
or
    return (last_ < rhs.begin_);

Also, I think that the implementation would be simpler and more efficient if
_end was chosen as the data member, rather than _last. As presently
implemented, there are quite a few calls to end(), which involves
calculating

    last_ + duration_rep::unit();

each time. I believe that if _end is chosen as the data member, then the
entire class can be implemented without any such calculations. The only one
would be in the implementation of last() itself, which would become

  template<class point_rep, class duration_rep>
  inline
  point_rep period<point_rep,duration_rep>::last() const
  {
    return  end_ - duration_rep::unit();
  }

Chris Trengove