Merge both Query structs into one / Reimplement query functions

[koenichiwa]

PrevFromQuery and NextAfterQuery are almost the same. This is also true for next_after(...) and prev_from(...) in Schedule. This seems to break with the DRY principle.

You could implement Query as follows:

pub enum QueryDirection {
    Next,
    Previous,
}

pub struct Query<Z> 
    where
    Z: TimeZone,
{
    initial_datetime: DateTime<Z>,
    first_month: bool,
    first_day_of_month: bool,
    first_hour: bool,
    first_minute: bool,
    first_second: bool,
    direction: QueryDirection,
}

impl<Z> Query<Z> 
where
    Z: TimeZone,
{
    pub fn from(date: &DateTime<Z>, direction: QueryDirection) -> Query<Z> {
        let new_date = match direction {
            QueryDirection::Next => date.clone() + Duration::seconds(1),
            QueryDirection::Previous => date.clone() - Duration::seconds(1)
        };
        Query {
            initial_datetime: new_date,
            first_month: true,
            first_day_of_month: true,
            first_hour: true,
            first_minute: true,
            first_second: true,
            direction: direction
        }
    }

    pub fn year_bound(&self) -> Ordinal {
        // Unlike the other units, years will never wrap around.
        self.initial_datetime.year() as u32
    }

    pub fn month_bound(&mut self) -> Ordinal {
        if self.first_month {
            self.first_month = false;
            return self.initial_datetime.month();
        }
        match &self.direction {
            QueryDirection::Next => Months::inclusive_min(),
            QueryDirection::Previous => Months::inclusive_max()
        }
    }

    pub fn reset_month(&mut self) {
        self.first_month = false;
        self.reset_day_of_month();
    }
    // etc..
}

This still leaves the functions breaking the DRY principle. You could also factor out Query and reimplement the functions with a combinator like so (NB this will currently fail the tests, but I think thats because of nanosecond issues)

    fn next_after<Z>(&self, after: &DateTime<Z>) -> Option<DateTime<Z>>
    where
        Z: TimeZone,
    {
        self.years()
            .iter()
            .skip_while(|year| *year < after.year() as u32)
            .flat_map(|year| {
                iter::repeat(after.with_year(year as i32))
                    .zip(self.months().iter())
            })
            .skip_while(|(date, month)| {
                date.as_ref().unwrap() < after ||
                (date.as_ref().unwrap() == after &&
                *month < after.month())
            })
            .flat_map(|(date, month)| {
                iter::repeat(date.unwrap().with_month(month))
                    .zip(self.days_of_month().iter())
            })
            .skip_while(|(date, day)| {
                date.as_ref().unwrap() < after ||
                (date.as_ref().unwrap() == after &&
                *day < after.day())
            })
            .map(|(date, day)| {
                date.unwrap().with_day(day)
            })
            .filter(|date| {
                self.days_of_week().includes(
                    date.as_ref().unwrap().weekday().number_from_sunday()
                )
            })
            .flat_map(|date| {
                iter::repeat(date).zip(self.hours().iter())
            })
            .skip_while(|(date, hour)| {
                date.as_ref().unwrap() < after ||
                (date.as_ref().unwrap() == after &&
                *hour < after.hour())
            })
            .flat_map(|(date, hour)| {
                iter::repeat(date.unwrap().with_hour(hour)).zip(self.minutes().iter())
            })
            .skip_while(|(date, minute)| {
                date.as_ref().unwrap() < after ||
                (date.as_ref().unwrap() == after &&
                *minute < after.minute())
            })
            .flat_map(|(date, minute)| {
                iter::repeat(date.unwrap().with_minute(minute)).zip(self.seconds().iter())
            })
            .skip_while(|(date, second)| {
                date.as_ref().unwrap() < after ||
                (date.as_ref().unwrap() == after &&
                *second <= after.second())
            })
            .map(|(date, second)| {
                date.unwrap().with_second(second)
            })
            .next()
            .flatten()
    }

This releaves the function of some indentation, and if you take a QueryDirection enum as an argument and slap some match clauses in there, you might be able to merge both functions together.

[koenichiwa]

Of course the unwrap statements in the function implementation should be changed. The signature would also have to change as follows:

pub enum QueryDirection {
    Next,
    Previous,
}
fn query<Z>(&self, date: &DateTime<Z>, direction: QueryDirection) -> Option<DateTime<Z>> // TODO: is query() descriptive enough?
    where
        Z: TimeZone,
    {
    // Implementation with match clauses
}

The draft that I made, as it stands, currently returns this error whilke testing:

thread 'schedule::test::test_next_and_prev_from' panicked at 'assertion failed: `(left == right)`
  left: `Some(2022-01-01T17:05:00Z)`,
 right: `Some(2022-01-01T17:05:00.513188Z)`', src/schedule.rs:396:9

So you'd have to do something like cleaning up those nanoseconds

[koenichiwa]

#80 is now a working draft. The two functions aren't merged yet, but it does clean up a bit of code as it stands. Queries.rs is deleted and there is less indentation.
Let me know if I should move forward in this direction.