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

[Jeff Garland]

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
>   inline
>   period::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
___
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost

[boost] Period calculations in Boost Date-Time

[Chris Trengove]

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
  inline
  period::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
  inline
  point_rep period::last() const
  {
return  end_ - duration_rep::unit();
  }

Chris Trengove



___
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost