time-rs / time

The most used Rust library for date and time handling.
https://time-rs.github.io
Apache License 2.0
1.13k stars 281 forks source link

Month::from_number is pub(crate) #563

Closed per-oestergaard closed 1 year ago

per-oestergaard commented 1 year ago

Hi. Before create like a pull request, I am curious to know why https://github.com/time-rs/time/blob/216d872e0251a0ed0bcd7cd5721a0a7bbde6fbf9/time/src/month.rs#L31 is not public outside the crate? Does anyone know the reason?

I am migrating some code from Chrono to Time. As I do not want to base my code on internal enum values, I am ending with code like this -

pub fn number_to_month(number: u8) -> Month {
    // Do not assume enum values match month values
    const M01: u8 = Month::January as u8;
    const M02: u8 = Month::February as u8;
    const M03: u8 = Month::March as u8;
    const M04: u8 = Month::April as u8;
    const M05: u8 = Month::May as u8;
    const M06: u8 = Month::June as u8;
    const M07: u8 = Month::July as u8;
    const M08: u8 = Month::August as u8;
    const M09: u8 = Month::September as u8;
    const M10: u8 = Month::October as u8;
    const M11: u8 = Month::November as u8;
    const M12: u8 = Month::December as u8;
    match number {
        M01 => Month::January,
        M02 => Month::February,
        M03 => Month::March,
        M04 => Month::April,
        M05 => Month::May,
        M06 => Month::June,
        M07 => Month::July,
        M08 => Month::August,
        M09 => Month::September,
        M10 => Month::October,
        M11 => Month::November,
        M12 => Month::December,
        _ => panic!("Month is invalid '{number}'"),
    }
}

Not that I ever need to update it, I guess. But still it would be great to avoid it and it might not be the most efficient code.

jhpratt commented 1 year ago

A note — you can rely on the representation, as Month is #[repr(u8)], with January being 1 and December being 12.

The reason this isn't public is because it uses NonZeroU8 and the ergonomics around nonzero types are not great. However, Month does impl TryFrom<u8>, so you can just do number.try_into() and get the Result back.

per-oestergaard commented 1 year ago

Thanks @jhpratt for the explanation.

I'll share my actual code for others to see.

So now it is -

let month : Month = number.try_into().unwrap();

or -

TryInto::<Month>::try_into(number).unwrap()

And if you do it often, you can define a trait -

trait FromNumber {
    fn from_number(number: u8) -> Month;
}

impl FromNumber for Month {
    fn from_number(number: u8) -> Month {
        TryInto::<Month>::try_into(number).unwrap()
    }
}

Month::from_number(number);

Much better :)

jhpratt commented 1 year ago

You can also bypass the extension trait and just use Month::try_from :)

per-oestergaard commented 1 year ago

Thx. You are right -

Month::try_from(number).unwrap()

When I look a little back, I ended here as I did not have a simple YMD method to create a date but had to use Date::from_calendar_date(year, month, day) where month is Month and I need a number. So I have to convert the number to Month to call from_calendar_date.