Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

std: add Duration::as_millis #28404

Closed
wants to merge 1 commit into from

Conversation

seanmonstar
Copy link
Contributor

Following "do whatever is easiest", this was about the same amount of work to implement vs write up an RFC.

Motivation: there already exists a Duration::from_millis function, but no mirror Duration::as_millis. Milliseconds are commonly desired in programming, and the lack of as_millis feels unbalanced.

@rust-highfive
Copy link
Collaborator

r? @aturon

(rust_highfive has picked a reviewer for you, use r? to override)

/// Returns the number of whole milliseconds represented by this duration.
#[unstable(feature = "duration_as_millis", reason = "newly added")]
pub fn as_millis(&self) -> u64 {
self.secs * MILLIS_PER_SEC + ((self.nanos / NANOS_PER_MILLI) as u64)
Copy link
Member

Choose a reason for hiding this comment

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

This can overflow, so should probably return an Option instead.

@seanmonstar
Copy link
Contributor Author

@sfackler updated to use Option

@alexcrichton
Copy link
Member

The RFC for Duration explicitly avoided this method:

This proposal does not include a method to get a number of milliseconds from a Duration, because the number of milliseconds could exceed u64, and we would have to decide whether to return an Option, panic, or wait for a standard bignum. In the interest of limiting this proposal to APIs with a straight-forward design, this proposal defers such a method.

As a result, I think that accessor methods like this may wish to go through an RFC. For example I would have questions along the lines of:

  • What about other units of time? Should we have as_foo for minutes, hours, microseconds, nanoseconds, etc?
  • Should there also be subsec_foo for microseconds and milliseconds?

@seanmonstar
Copy link
Contributor Author

@alexcrichton This is motivated by balancing the accessors with the constructors: there exists from_millis but no as_millis. As there aren't any from_mins, from_hours, etc, they seem out of scope.

But I definitely can see how the color of this shed could be rainbow, and I can write a function to do the same thing for my needs.

@alexcrichton
Copy link
Member

I understand the motivation, but I don't personally think that it's strong enough to bypass an RFC when the original RFC explicitly avoided this.

@seanmonstar seanmonstar deleted the duration-as-millis branch October 1, 2015 20:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants