Opened 11 years ago

Closed 9 years ago

#5753 closed Bugs (invalid)

previous_weekday() returns the very same day

Reported by: mjklaim@… Owned by: az_sw_dude
Milestone: To Be Determined Component: date_time
Version: Boost 1.44.0 Severity: Problem
Keywords: Cc:

Description

We have a bug that seems to comes from boost::date_time::previous_weekday() (probably next_weekday() too) usage in case we want to go from a weekday to another same weekday:

#include <iostream>
#include <boost/date_time.hpp>
#include <boost/date_time/gregorian/gregorian.hpp>

 int main()
{
	using namespace boost::gregorian;
	auto today = date( 2011, Jul, 12 );
	
	auto last_tuesday = boost::date_time::previous_weekday( today,
greg_weekday(boost::date_time::Tuesday) );


	std::cout << today << '\n';
	std::cout << last_tuesday;
	std::cin.ignore();

	return 0;
}

This example runs but return the same date twice instead of giving the tuesday before the given date (that is already a tuesday). Compiled with VS2010SP1 (boost 1.46.1) and GCC4.4(boost 1.44.0 - didn't see the difference in the code) What we first supposed was it will go to the tuesday of the previous week if we already are tuesday. It appears to not work this way.

Is it the wanted behaviour? If it is, I suggest the documentation to be enhanced to warn the user about the case when you provide a date that is the same provided weekday.

The code of boost::date_time::previous_weekday() (as far as I understand it) suggests that it should go to the previous week's weekday instead of returning the same day:

template<class date_type, class weekday_type>
  inline
  date_type previous_weekday(const date_type& d, const weekday_type& wd)
  {
    return d - days_before_weekday(d, wd);
  }


and then :

template<typename date_type, class weekday_type>
  inline
  typename date_type::duration_type days_before_weekday(const
date_type& d, const weekday_type& wd)
  {
    typedef typename date_type::duration_type duration_type;
    duration_type wks(0);
    duration_type dd(wd.as_number() - d.day_of_week().as_number()); // 1
    if(dd.days() > 0){ // 2
      wks = duration_type(7);
    }
    // we want a number of days, not an offset. The value returned must
    // be zero or larger.
    return (-dd + wks); // 3
  }


Correct me if I'm wrong : 2. checks if the generated day is the same weekday than the provided one, right? So if I'm correct, there might be a problem in 1 or 3?

Change History (1)

comment:1 by Marshall Clow, 9 years ago

Resolution: invalid
Status: newclosed

Here's my take on the code in days_before_weekday:

1: Calculates a number in the range 0 .. 6 for the next time the specified day of the week rolls around. Note that asking for Tuesday on Tuesday gives you 0.

2: If we got a number > 0, then we're looking into the future. Set the wks variable to 7.

3: Step wks days into the future, and then subtract off the number of days we calculated in step 1.

Note that the comment at the end of the routine explicitly says that this function can return zero (i.e, zero days from d). I don't see a bug in this code (though I would be tempted to write it like this - warning: untested code):

    typedef typename date_type::duration_type duration_type;
    duration_type dd(wd.as_number() - d.day_of_week().as_number()); // 1
    return dd ? duration_type(7) - dd ? 0;

I don't think there's a bug in this code. The behavior "return 0 when asking for last Tuesday on Tuesday" is quite deliberate. Whether or not that should be the behavior is a different question, and that's a discussion that should be had on the boost mailing list.

I'll take a look at the documentation; see what the best place to add a note is. Closing this as "not a bug". If you disagree, please re-open it.

Note: See TracTickets for help on using tickets.