Skip to content
This repository has been archived by the owner on Apr 2, 2018. It is now read-only.

Adds new function interval_range #33

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

bkchr
Copy link

@bkchr bkchr commented Sep 23, 2017

The functions returns a Stream that fires at some random value, that is in the
given range, into the future. The next duration is calculated every time after
the Interval fired into the future, by choosing a random value in the given range.

@bkchr
Copy link
Author

bkchr commented Sep 23, 2017

Ahh I see that the tests are still run for rust 1.14, is this still a valid target? So should I remove the pub(crate)?

src/interval.rs Outdated
let mut rng = thread_rng();

let secs = rng.gen_range(min.as_secs(), max.as_secs() + 1);
let nsecs = rng.gen_range(min.subsec_nanos(), max.subsec_nanos() + 1);

Choose a reason for hiding this comment

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

This doesn't look right. For example if min = Duration::from_millis(1800), max = Duration::from_millis(2900) it will never generate Duration::from_millis(1950) or Duration::from_millis(2100)

Copy link
Author

@bkchr bkchr Oct 13, 2017

Choose a reason for hiding this comment

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

Yeah I see that there is a logic bug, but I don't understand that the values you are given, can not be generated? Even with the bug, these values should be generated.

Edit: Ohh yeah, I see what you mean.

Copy link
Author

Choose a reason for hiding this comment

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

Thanks for making me aware of this bug :)

The functions returns a `Stream` that fires at some random value, that is in the
given range, into the future. The next duration is calculated every time after
the Interval fired into the future, by choosing a random value in the given range.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants