Re: [boost] Period calculations in Boost Date-Time

From: Jeff Garland (jeff_at_[hidden])
Date: 2003-08-19 08:49:31

On Tue, 19 Aug 2003 13:25:37 +1000, Chris Trengove wrote
> 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) 

Yes, the function defintion is misleading.  In the function implementation
(shown above) it is clear that this is 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;

Yep. Fixed in CVS.
 
> 2) operator<() is implemented as
> 
>     return (last_ <= rhs.begin_);
> 
> and this should be
> 
>     return (end() <= rhs.begin_);
> or
>     return (last_ < rhs.begin_);

Yep. Fixed in CVS.

> Also, I think that the implementation would be simpler and more 
> efficient if _end was chosen as the data member, rather than _last. 
>
>  ... detail omitted...

Probably true, I'll put this on the todo list to look at.

Thanks for the detailed comments.

Jeff