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

Float ranges #62

Merged
merged 5 commits into from
Jan 7, 2019
Merged

Float ranges #62

merged 5 commits into from
Jan 7, 2019

Conversation

Stratus3D
Copy link
Contributor

@Stratus3D Stratus3D commented Dec 28, 2018

Update the range generator function so it checks the input range and generates a random float or random integer based on the types of the input numbers.

  • With two integers, a random integer within range is returned
  • With a float and an integer, a random float within range is returned
  • With two floats, a random float within range is returned

Fixes #60

Copy link
Member

@Ana06 Ana06 left a comment

Choose a reason for hiding this comment

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

@Stratus3D thanks for the PR! 💘

It would be great if you could add this change to the CHANGELOG. 🙏

Can you please remove e69ecd4 from this PR? It is already in #61

Could you please also mention #60 in the commit message:

Fixes https://github.com/rantly-rb/rantly/issues/60

@@ -158,7 +158,13 @@ def float(distribution=nil, params={})
def range(lo=nil,hi=nil)
lo ||= INTEGER_MIN
hi ||= INTEGER_MAX
rand(hi+1-lo) + lo

if lo.is_a?(Integer) && hi.is_a?(Integer)
Copy link
Member

Choose a reason for hiding this comment

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

I think a better solution would be using a range in rand: rand(lo..hi) 😉

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Strangely rand(lo..hi) doesn't work. I don't know why it was written this way originally, but I've left it as it is.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Introduced in commit 15a4eb0.

Copy link
Member

Choose a reason for hiding this comment

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

@Stratus3D what do you exactly mean with it doesn't work? 😕

Copy link
Member

Choose a reason for hiding this comment

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

@Stratus3D I think you didn't see my comment, what do you exactly mean with it doesn't work? can you please elaborate?

test/rantly_generator_test.rb Outdated Show resolved Hide resolved
test/rantly_generator_test.rb Outdated Show resolved Hide resolved
test/rantly_generator_test.rb Outdated Show resolved Hide resolved
test/rantly_generator_test.rb Outdated Show resolved Hide resolved
test/rantly_generator_test.rb Show resolved Hide resolved
Copy link
Member

@Ana06 Ana06 left a comment

Choose a reason for hiding this comment

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

@Stratus3D can you please rebase? I have introduced Rubocop in #66 and I would like to avoid introducing new offenses in this PR. Sorry for that 😢

@Stratus3D
Copy link
Contributor Author

PR updated! I fixed all the rubocop warnings.

Copy link
Member

@Ana06 Ana06 left a comment

Choose a reason for hiding this comment

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

I am not conviced with not using rand(lo..hi), but it can be fixed later on. Let 🚢 it! Thanks for the contribution @Stratus3D 💘

@@ -158,7 +158,13 @@ def float(distribution=nil, params={})
def range(lo=nil,hi=nil)
lo ||= INTEGER_MIN
hi ||= INTEGER_MAX
rand(hi+1-lo) + lo

if lo.is_a?(Integer) && hi.is_a?(Integer)
Copy link
Member

Choose a reason for hiding this comment

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

@Stratus3D I think you didn't see my comment, what do you exactly mean with it doesn't work? can you please elaborate?

@Ana06 Ana06 merged commit 30f3b7e into rantly-rb:master Jan 7, 2019
@Stratus3D Stratus3D deleted the float-ranges branch January 7, 2019 13:58
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.

2 participants