rust-datetime / datetime

Dates and times library for Rust.
MIT License
15 stars 9 forks source link

Month enum does not coerce into i32 nicely #3

Closed hoodie closed 9 years ago

hoodie commented 9 years ago

Hello,

do you think it would cause breakage if Month was represented in this way:

pub enum Month {
    January   = 1,
    February  = 2,
    March     = 3,
    April     = 4,
    May       = 5,
    June      = 6,
    July      = 7,
    August    = 8,
    September = 9,
    October   = 10
    November  = 11,
    December  = 12,
}

This would allow it to be coerced into a an i32 more nicely without manually having to add 1 to get the actual number of the month. Your doc comment mentions the implementation as enum instead of as a number, but does not explicitly state why this wouldn't work to.

Thank you

PS: of course this would break everybody's code who have already manually added 1 to the month, but I would consider this acceptable for a 0.3.x release.

ogham commented 9 years ago

First off, sorry about the dire state of the documentation at present! It's something I need to work on.

You're right that this would cause breakage, but I'm not sure of the social contracts for using month as i32 in your code when Months don't actually have values assigned to them. To be honest, I can't remember when this behaviour was added - I always thought it was a compiler error to try to coerce an enum that didn't have values.

Anyway, I'd be up for adding this, and doing a 0.4.0 release to mark it. Do you have a use case for when you need the 'natural' month numbers?

hoodie commented 9 years ago

Thank you, that sounds good. My use for 'natural' month numbers is when speaking to an API that takes year, month and day separately as integers. In that case my solution looks like this:

//...
                month            = route.time.month() as i32 + 1, // january = 0
//...

which feels a little clumsy.

ogham commented 9 years ago

Fair enough! Added in e022973cc946f29c78544c1e028dfaec7c1032e3 along with a version bump.