-
Notifications
You must be signed in to change notification settings - Fork 179
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
Added numberBetween for doubles. #1392
Conversation
PR Summary
|
datafaker/src/test/java/net/datafaker/FakerTest.java Lines 211 to 215 in 12bdd2a
|
Interesting. What should we do with this? I think what I'm basically asking is: |
Yes in this case it could be. |
I agree. I'm just not sure if that's going to break anything. Probably not likely, but not sure. Any suggestions on what to do here? Reject the PR, fix the test, something else? |
I see two options.
|
public double numberBetween(double min, double max) { | ||
return faker.random().nextDouble(min, max); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it looks like nothing prevents from calling it like
faker.number().numberBetween(123.456, 1.0);
what is expected output of it?
based on name the result should be somewhere between the boudaries
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The same as what the other methods return.
faker.number().numberBetween(10, 0)
Also returns a number between 0 and 10.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
would be great to have a test for this case as well
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fair enough, I added it.
I did. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1392 +/- ##
============================================
- Coverage 92.26% 92.06% -0.20%
+ Complexity 3166 3157 -9
============================================
Files 320 320
Lines 6177 6178 +1
Branches 592 592
============================================
- Hits 5699 5688 -11
- Misses 335 344 +9
- Partials 143 146 +3 ☔ View full report in Codecov by Sentry. |
No description provided.