Export days_in_year_month

[mvaled]

This function is currently private, but it can be useful for validation purposes and arbitrary (quickcheck generation of valid dates.

[jhpratt]

The current implementation is for internal usage, so valid data is assumed. If I were to make this public, it would have to have additional checks for the bounds in both year and month. The bounds checks would necessitate the possibility of either a panic or returning a Result. I definitely don't want to do the latter (due to current internal usage), and panicking typically isn't a good thing. This is also kind of an implementation detail, which I tend to avoid exposing (see how some methods are not const fn, for example).

Documentation would also have to be added, but that wouldn't be difficult.

I'll have to think about this more, but it would require some fiddling around to make things work.

[mvaled]

Hi @jhpratt,

I could (with some guidance since I'm only beginning to grasp Rust) work on this in PR #220. I would follow the same pattern in exported functions days_in_year, and weeks_in_year.

[mvaled]

To address the concerns about valid data we could rather implement try_days_in_year_month with signature:

/// Get the number of days in the month of a given year.
pub const fn try_days_in_year_month(year: i32, month: u8) -> Result<u8, ComponentRangeError> {
   ...
}

[jhpratt]

Like I said, I'll take a look into things. I'm just not 100% on whether this would be necessary if the internal structure of Date were to change. days_in_year and weeks_in_year only require the year, which is technically only supported in the range -100_000..=100_000. However, the methods work fine on data outside that range. days_in_year_month inherently cannot be the same, as there's only 12 months in a year.

With regard to a try_… method, as I mentioned previously it's possible, but I would prefer to avoid that. Internal usage assumes valid data, and I don't like littering .expect(…) where it's not necessary.

[mvaled]

Like I said, I'll take a look into things.

Ok. I'll wait for your comments.

[mvaled]

Just to comment about my actual need. I was trying to generate arbitrary but valid instances of Date using quickcheck. The core of the implementation was something like:

#cfg(test)
mod test {
    #[derive(Debug, Clone)]
    struct DateBuilder {
        year: i32,
        month: u8,
        day: u8
    }
    impl Arbitrary for DateBuilder {
        fn arbitrary<G: Gen>(g: &mut G) -> Self {
            let year: i32 = g.gen_range(-100_000, 100_001);
            let month: u8 = g.gen_range(1, 13);
            let day: u8 = g.gen_range(1, days_in_year_month(year, month) + 1);
            DateBuilder{year: year, month: month, day: day}
        }
    }
    impl DateBuilder {
        fn get_date(&self) -> Date {
            Date::try_from_ymd(self.year, self.month, self.day).unwrap()
        }
    }

   // etc...
}

You may notice that I needed to use days_in_year_month to generate valid values for days. I don't know if you'd be willing to have a feature "with_quickcheck" to have the implementation of Arbitrary right in the crate time without needing to do the DateBuilder intermediate struct.

[jhpratt]

Is there any particular reason you're using quickcheck over proptest? At least from this issue, it seems like proptest would be preferred in the vast majority of cases.

I could conceivably implement rand::distributions::Distribution, which would be even more generic. That would let you get an arbitrary value as well.

[mvaled]

Is there any particular reason you're using quickcheck over proptest?

Plain ignorance. I didn't know about proptest. Now that you have introduced it to me, I think I will like it better (I have extensively used Hypothesis for Python). I searched for 'quickcheck' because that's the "original" Haskell implementation.

[jhpratt]

Going ahead with implementation of rand::distributions::Distribution on various types. For the record, you can currently do what you want by using Date::try_from_yo in conjunction with days_in_year.

[mvaled]

Great! Looking forward a new release of time. ;)

I'm reading the docs of proptest. Thanks for the tip on composing try_from_yo and days_in_year. It's really straightforward.

[jhpratt]

As the provided use case can be accomplished other ways and support for the rand crate is now on master, closing this.