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

Fix random() #207

Merged
merged 3 commits into from Aug 17, 2019

Conversation

@bmpGames
Copy link
Contributor

commented Jul 2, 2019

Fix issue #206

@coveralls

This comment has been minimized.

Copy link

commented Jul 2, 2019

Coverage Status

Coverage remained the same at 100.0% when pulling 498c2fa on bmpGames:fix-random into 97b8127 on you-dont-need:master.

2 similar comments
@coveralls

This comment has been minimized.

Copy link

commented Jul 2, 2019

Coverage Status

Coverage remained the same at 100.0% when pulling 498c2fa on bmpGames:fix-random into 97b8127 on you-dont-need:master.

@coveralls

This comment has been minimized.

Copy link

commented Jul 2, 2019

Coverage Status

Coverage remained the same at 100.0% when pulling 498c2fa on bmpGames:fix-random into 97b8127 on you-dont-need:master.

@coveralls

This comment has been minimized.

Copy link

commented Jul 2, 2019

Coverage Status

Coverage remained the same at 100.0% when pulling 8c035ce on bmpGames:fix-random into 97b8127 on you-dont-need:master.

@stevemao stevemao requested review from cylim and dennisXZX Jul 2, 2019

README.md Outdated Show resolved Hide resolved

@cht8687 cht8687 self-requested a review Aug 9, 2019

@cht8687

cht8687 approved these changes Aug 9, 2019

Copy link
Member

left a comment

LGTM

@cylim
Copy link
Member

left a comment

_.random(1,0)  // return 1 or 0,
random(1, 0) // always return 1

_.random(100,99)  // return 100 or 99,
random(100, 99) // always return 100
@bmpGames

This comment has been minimized.

Copy link
Contributor Author

commented Aug 10, 2019

@cylim @stevemao
I just ran my random on lodash tests and.. I give up. Lodash do some crazy stuff, like:

  1. _.random() is just 0 or 1, but IMHO it needs to act like Math.random()
  2. coercion! _.random(NaN), _.random(Infinity)
  3. random(true), another if in code? no, thanks
  4. array.map(random), OH NO NO NO

I give up and let's write just 2 functions?
random() for floats and randomInt() for integers

@bmpGames

This comment has been minimized.

Copy link
Contributor Author

commented Aug 10, 2019

@cht8687 @cylim @stevemao, what do you think?
Now it seems neat and clean 😌

@cylim cylim requested review from stevemao and cht8687 Aug 11, 2019

@stevemao
Copy link
Member

left a comment

Yeah sounds good :)

@cht8687 cht8687 merged commit 93eee15 into you-dont-need:master Aug 17, 2019

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage remained the same at 100.0%
Details

@bmpGames bmpGames deleted the bmpGames:fix-random branch Aug 17, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
6 participants
You can’t perform that action at this time.