ganon icon indicating copy to clipboard operation
ganon copied to clipboard

Fix randomInRange method

Open emilyanndavis opened this issue 7 years ago • 4 comments

There is currently a method called randomInRange that lives in /lib/randomInRange.js. It is incomplete and needs to be fixed!

randomInRange returns a random integer that falls within a given range, inclusive of the min and max integers provided. For example, randomInRange(10, 15) could return 10, 11, 12, 13, 14, or 15.

Acceptance Criteria:

  1. Running yarn test randomInRange results in tests passing.
  2. You have written a skeleton method for someone else to work on.
  3. You have written tests surrounding your skeleton method.
  4. Running yarn lint does not print any errors to the console!
  5. Optional: write one or two more tests surrounding the method you are solving to account for potential edge cases.

Please include the skeleton of a new method + an accompanying test for someone else to work on at the time of creating a pull request! A pull request will most likely be denied if it does not contain a skeleton method for someone else to work on! For more information, please read the Contributing Guide.

Thank you so much for your contribution!

emilyanndavis avatar Nov 02 '17 05:11 emilyanndavis

@BlakeGuilloud I would like to work on this

jmknoll avatar Nov 02 '17 09:11 jmknoll

@emilyanndavis What are you testing for with this assertion?

expect(() => randomInRange(1.0, 10)).toThrow()

javaScript doesn't differentiate between floats and ints, so for all intents and purposes 1.0 === 1

Or is there something that I'm missing here?

jmknoll avatar Nov 03 '17 12:11 jmknoll

Oops. You're absolutely right @jmknoll ! That test was supposed to make sure the first param is an integer but, as you've pointed out, JavaScript doesn't care whether it's 1 or 1.00000. Feel free to change that 1.0 to something that won't be evaluated as equivalent to an integer. Good catch!

emilyanndavis avatar Nov 03 '17 17:11 emilyanndavis

All good. I had already commented out the test by the time I submitted the PR implementing this function, but I think the other parameter tests are sufficient enough to cover for this one.

jmknoll avatar Nov 04 '17 04:11 jmknoll