Skip to content

Conversation

esheppa
Copy link
Collaborator

@esheppa esheppa commented Nov 23, 2022

These functions are to reduce the number of unwraps needed when creating NaiveDates

Copy link
Contributor

@jtmoon79 jtmoon79 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These look convenient!

Please add #[must_use] to these functions.

@pitdicker
Copy link
Collaborator

pitdicker commented Apr 28, 2023

I am not super convinced of the usefulness of all these methods. My reason is that there are all kinds of operations someone may want to do on dates. Where do you draw the line?

end_month and end_year seem useful because they are non-trivial. You should get the current year/month, add 1 (this can cause a panic on overflow), and get the preceding day.

But then you would also want start_month and start_year for symmetry, which don't feel valuable enough for me.

Would similar methods on NaiveDateTime or DateTime make sense? Would the end of a month be at the last second? Or the last nanoseond? Or maybe even leap-second? Strictly speaking, the end is the same as the start of the next day, at 0:00:00. But than the date would be different from the one returned by NaiveDate::end_month.

Self::from_ymd_opt(year.into(), month.number_from_month(), 1).unwrap()
}

/// Create a `NaiveDate` of the ;ast day of the given year and month
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
/// Create a `NaiveDate` of the ;ast day of the given year and month
/// Create a `NaiveDate` of the last day of the given year and month

///
/// This takes an `i16` rather than `i32` to ensure it is not
/// out of the allowed range
pub fn start_year(year: i16) -> NaiveDate {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using an i16 ties into your work in #882. But maybe it is best to accept an i32 until the decision is made to restrict the range of allowed years?

/// out of the allowed range
pub fn end_year(year: i16) -> NaiveDate {
// ideally this unwrap can be later removed
Self::from_ymd_opt(i32::from(year) + 1, 1, 1).unwrap().pred_opt().unwrap()
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This might fail if year == MAX_YEAR.

// ideally this unwrap can be later removed
let start = Self::start_month(year, month);

(start + Months::new(1)).pred_opt().unwrap()
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This might fail if year == MAX_YEAR && month == 12.

@pitdicker
Copy link
Collaborator

Closing. I would be interested to have a method to easily create a NaiveDate at the end of a month.

@pitdicker pitdicker closed this Apr 10, 2024
@pitdicker pitdicker deleted the add-date-helpers branch April 10, 2024 12:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants