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

Jump to bottom

allow rand with zero value #3493

Merged
merged 1 commit into from Sep 10, 2017
Merged

allow rand with zero value #3493

merged 1 commit into from Sep 10, 2017

Conversation

kostya
Copy link
Contributor

@kostya kostya commented Nov 1, 2016

this is useful in such code:

def delay(interval : Float64)
  sleep(rand(interval))
end

# when you want just not to sleep
delay(0.0)

@RX14
Copy link
Contributor

RX14 commented Nov 1, 2016

This is quite useful but i'm concerned that it could mask logic bugs in code that would otherwise raise.

@kostya
Copy link
Contributor Author

kostya commented Nov 1, 2016 •
edited

why not?
also it already works for range: rand(0..0) => 0, rand(0.0..0.0) => 0.0, why not to work for zero itself?

@trans
Copy link

trans commented Nov 8, 2016

This should definitely work. It provides an inherit on/off behavior, removing the need for extra conditionals, allowing code to be more succinct.

but i'm concerned that it could mask logic bugs in code that would otherwise raise.

That's what tests are for. Besides, if you are expecting behavior based on the generation of random numbers, it's usually pretty obvious when you just keep getting zeros.

@kostya
Copy link
Contributor Author

kostya commented Dec 15, 2016

👍

@straight-shoota
Copy link
Member

@kostya This PR needs a rebase. Otherwise it looks good to me

@kostya
Copy link
Contributor Author

kostya commented Jun 29, 2017

rebased

@Sija
Copy link
Contributor

Sija commented Sep 10, 2017

@asterite @mverzilli @RX14 🏓

@asterite asterite merged commit 6202e37 into crystal-lang:master Sep 10, 2017
@RX14 RX14 added this to the Next milestone Sep 10, 2017
@oprypin oprypin mentioned this pull request Sep 8, 2018
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@asterite asterite asterite approved these changes

@RX14 RX14 RX14 approved these changes

Assignees
No one assigned
Labels
kind:feature topic:stdlib
Projects
None yet
Milestone
0.24.0
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@kostya @RX14 @trans @straight-shoota @Sija @asterite