Skip to content

Duration: methods to extract non-integral content #18855

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

Closed
aturon opened this issue Nov 10, 2014 · 7 comments
Closed

Duration: methods to extract non-integral content #18855

aturon opened this issue Nov 10, 2014 · 7 comments

Comments

@aturon
Copy link
Member

aturon commented Nov 10, 2014

Right now, Duration::num_seconds and friends return i64 values, which means you get the number of whole seconds included in the duration. But it's often useful to get fractional values as well.

It's unclear whether the methods should simply be changed to yield f64 values, or whether we should have separate variants.

@aturon aturon added the A-libs label Nov 10, 2014
@aturon
Copy link
Member Author

aturon commented Nov 10, 2014

cc @alexcrichton

What's the general precedent here?

@aturon
Copy link
Member Author

aturon commented Nov 10, 2014

cc @brson

@alexcrichton
Copy link
Member

My use case was when I had a duration of time and I wanted to print out how many "things per second" happened in that duration of time. Given a number of "things", I would ideally use a f64 number of seconds to print a more precise amount, but I was unable to do so without acrobatics of calling around num_nanoseconds() and then remember whether to divide or multiple by the number of nanoseconds in a second.

I'm not sure that we want to s/i64/f64/ in the API, just a thought of mine that perhaps it could be a little easier to extract the float-wise number of units.

@1-more
Copy link
Contributor

1-more commented Nov 11, 2014

Let's make Duration fields public. Then everyone will be able to take what he/she wants. It'll also address this issue.

@1-more
Copy link
Contributor

1-more commented Nov 11, 2014

I don't like separate variants. If we can't make Duration fields public, then let's export internals with some function:

impl Duration {
    ...    
    pub fn as_is(&self) -> (i64, i32) {
        (self.secs, self.nanos)
    }
}

and use macros:

macro_rules! f64_milliseconds (
    ($d:expr) => {
        match $d.as_is() {
            (secs, nanos) => (secs as f64)*1000f64 + (nanos as f64)/1000_000f64
        }
    }
)

macro_rules! f64_microseconds (
    ($d:expr) => {
        match $d.as_is() {
            (secs, nanos) => (secs as f64)*1000_000f64 + (nanos as f64)/1000f64
        }
    }
)

@1-more
Copy link
Contributor

1-more commented Nov 12, 2014

Yesterday we were happy with Duration::num_seconds() -> i64. Today we need Duration::num_seconds() -> f64. That's the way life goes. But what is going to happen tomorrow? I like the idea to leave Duration alone and move num_seconds function in a trait:

pub trait TimeSpan {
    fn num_seconds(x: &Duration) -> Self;
}

Then, implement trait for both i64 and f64:

impl TimeSpan for i64 {
    fn num_seconds(x: &Duration) -> i64 {
        x.num_seconds()
    }
}

impl TimeSpan for f64 {
    fn num_seconds(x: &Duration) -> f64 {
        (x.num_seconds() as f64) + (x.nanos as f64)/(NANOS_PER_SEC as f64)
    }
}

And finally use it:

let d = Duration::milliseconds(1500);

let a: i64 = TimeSpan::num_seconds(&d);
assert_eq!(a, 1);

let b: f64 = TimeSpan::num_seconds(&d);
assert_eq!(b, 1.5);

In general, it seems to be a more elegant solution. Though, a single Duration::num_seconds() -> f64 is easier.

alexcrichton added a commit to alexcrichton/rust that referenced this issue Nov 12, 2014
This commit deprecates the entire libtime library in favor of the
externally-provided libtime in the rust-lang organization. Users of the
`libtime` crate as-is today should add this to their Cargo manifests:

    [dependencies.time]
    git = "https://github.com/rust-lang/time"

To implement this transition, a new function `Duration::span` was added to the
`std::time::Duration` time. This function takes a closure and then returns the
duration of time it took that closure to execute. This interface will likely
improve with `FnOnce` unboxed closures as moving in and out will be a little
easier.

Due to the deprecation of the in-tree crate, this is a:

[breaking-change]

cc rust-lang#18855, some of the conversions in the `src/test/bench` area may have been a
little nicer with that implemented
bors added a commit that referenced this issue Nov 12, 2014
This commit deprecates the entire libtime library in favor of the
externally-provided libtime in the rust-lang organization. Users of the
`libtime` crate as-is today should add this to their Cargo manifests:

    [dependencies.time]
    git = "https://github.com/rust-lang/time"

To implement this transition, a new function `Duration::span` was added to the
`std::time::Duration` time. This function takes a closure and then returns the
duration of time it took that closure to execute. This interface will likely
improve with `FnOnce` unboxed closures as moving in and out will be a little
easier.

Due to the deprecation of the in-tree crate, this is a:

[breaking-change]

cc #18855, some of the conversions in the `src/test/bench` area may have been a
little nicer with that implemented
@alexcrichton
Copy link
Member

The Duration type has been redesigned since this was opened, so I'm going to close this as stale (as it basically no longer applies)

lnicola pushed a commit to lnicola/rust that referenced this issue Jan 20, 2025
internal: Migrate `if let` replacement assists to `SyntaxEditor`
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

No branches or pull requests

3 participants