Skip to content

Instantly share code, notes, and snippets.

@brandonhilkert
Last active June 12, 2018 14:48
  • Star 4 You must be signed in to star a gist
  • Fork 2 You must be signed in to fork a gist
Star You must be signed in to star a gist
Save brandonhilkert/7848392 to your computer and use it in GitHub Desktop.
class MegaLotto
NUMBERS = 5
def draw
NUMBERS.times.inject([]) do |arr, i|
arr << single_draw
end
end
private
def single_draw
rand(0...60)
end
end
MegaLotto.new.draw # => [23, 22, 3, 7, 16]
@dleve123
Copy link

I think you meant to write << single_draw considering the method definition of L12.

@bf4
Copy link

bf4 commented Dec 12, 2013

Also better to use each_with_object([])

@dariye
Copy link

dariye commented Dec 12, 2013

I think inject is fine tbh. I noticed the random_draw typo as well.

@erich
Copy link

erich commented Dec 12, 2013

@chip
Copy link

chip commented Dec 14, 2013

Also, I think line 17 should be this instead:

MegaLotto.draw

Otherwise, the class needs to be renamed on line 1 to LotteryDrawing.

@abinoam
Copy link

abinoam commented Dec 14, 2013

@chip

MegaLotto.new.draw

@s2k
Copy link

s2k commented Dec 14, 2013

Hi all,
apart from the naming issues, I find the use of inject to be too complicated for the relatively simple thing the code is doing. At first I thought

    def draw
      (1..NUMBERS).map{single_draw}
    end

is a better solution, but the code is still creating a superfluous object (the (1..NUMBER) range).
In the end, I think the code should 'just' do what it's supposed to do: Create an Array of NUMBERS elements each of which is initialised in a certain way.
Actually there's a method for that: Array.new:

    def draw
      Array.new(NUMBERS){single_draw}
    end

@s2k
Copy link

s2k commented Dec 14, 2013

Additional remark: Using Array.new also seems to be the fastest of the the ways:

class MegaLotto
  NUMBERS = 5

 def draw_inject
    NUMBERS.times.inject([]) do |arr, i| 
      arr << single_draw
    end
  end

  def draw_map
    (1..NUMBERS).map{single_draw}
  end

  def draw_new
    Array.new(NUMBERS){single_draw}
  end

  private

  def single_draw
    rand(0...60)
  end
end

require 'benchmark'

N = 500000
ml = MegaLotto.new
Benchmark.bm do | x |
  x.report('inject:'){ N.times{ ml.draw_inject }}
  x.report('map   :'){ N.times{ ml.draw_map }}
  x.report('new   :'){ N.times{ ml.draw_new }}
end

On my machine:

         user       system     total       real
inject:  1.590000   0.000000   1.590000 (  1.598250)
map   :  1.230000   0.010000   1.240000 (  1.234696)
new   :  1.030000   0.000000   1.030000 (  1.024241)

Copy link

ghost commented Dec 15, 2013

Guys, this is about writing a gem, not about writing the bestest ever random lottery number generator. other than the few typos, the focus is on the ability to write a gem not a generator.

@brandonhilkert
Copy link
Author

Ha! Thanks for all the feedback. Sorry for the typos...I originally wrote this for something else, so the class/method names were a little off.

The point of the course was not about the code (I think I mentioned that in the course). Feel free to write whatever code you like. I can't imagine this code being very useful anyway, so hopefully you're writing something that you'll benefit from. I wanted to get something down to discuss the structure and use of a gem, but again, feel free to use whatever code you like.

If you prefer your implementation of the code, feel free to use it.

@sonianand11
Copy link

sounds good to me. looking froward for next session of course 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment