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

[Crystal] Add Lucky Framework #6069

Merged
merged 1 commit into from
Oct 12, 2020

Conversation

matthewmcgarvey
Copy link
Contributor

Adding Lucky which is a Crystal framework.

Was kept from running tests by #6065 unless using changes in #6067

@@ -0,0 +1,6 @@
class Db::Index < BaseAction
get "/db" do
world = WorldQuery.find(rand(1..ID_MAXIMUM))
Copy link
Member

Choose a reason for hiding this comment

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

I vaguely remember taking a dive into the Crystal codebase and seeing that the rand(range) wasn't inclusive on the upper bounds. I could be wrong, but want to confirm that this could return 10,000.

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, I remember now. The documentation wasn't clear so I had to dig through the code: https://crystal-lang.org/api/0.24.2/Random.html#rand%28range%3ARange%28Int%2CInt%29%29%3AInt-instance-method

https://github.com/crystal-lang/crystal/blob/4f9ed8d03208dd0db33993c5a6fa6753bc1cf91e/src/random.cr#L219 Seems in this case it's basically doing:

range.begin + rand(range.end - range.begin) and rand(int) is not inclusive, so the max of : 1 + rand(9999) is 9999

Copy link
Contributor Author

Choose a reason for hiding this comment

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

admittedly I copied it from the other crystal projects so it definitely could have been wrong but I ran a little test in a repl calling rand(1..3) and was able to get a 3. https://play.crystal-lang.org/#/r/9toe

Copy link
Member

Choose a reason for hiding this comment

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

wow that is interesting. the source code in the crystal repo reads to me like that would be doing:

puts (1..10_000).map { 1 + rand(3 - 1) }.uniq

But the REPL says otherwise lol, thanks!

@NateBrady23 NateBrady23 merged commit 3cfcda1 into TechEmpower:master Oct 12, 2020
@matthewmcgarvey matthewmcgarvey deleted the add-lucky branch October 12, 2020 21:17
tomciopp pushed a commit to tomciopp/FrameworkBenchmarks that referenced this pull request Oct 21, 2020
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.

None yet

2 participants