Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

I created a quick program to test the precision of randomness generated by the .rand method. The primary question is whether I can use the .times method in place of the while code blocks to increase efficiency and:

  1. Whether such a practice would reduce the amount of processing required/if this would be at all significant.
  2. Whether it's a more common approach, or alternatively is it an infeasible/awkward approach.
  3. Even if it is a less acceptable approach than the one I've taken, how would I use the .times method to execute the same task. If it is inappropriate when should I use .times?

    # Initializes array and iterator.
    x = []
    i = 0
    
    # Stores 4000 random numbers 0..1 in array 'x'
    while i < 4000
      x << rand(2)
      i += 1
    end
    
    # Initializes array for the purpose of storing zeros and ones in separate
    #  arrays for the sake of counting how many instances of each occur in the sample
    count_one = []
    count_zero = []
    
    # Resets iterator to 0
    i = 0
    
    # Stores instances of zero in `count_zero` and instances of one in `count_one`
    while i < x.length
      if x[i] == 1
        count_zero << x[i]
        i += 1
      else
        count_one << x[i]
        i += 1
      end
    end
    
    # Calculates final averages
    zero_average = count_zero.length.to_f/x.length.to_f * 100.0    
    one_average = count_one.length.to_f/x.length.to_f * 100.0
    

Additionally, I am curious as to:

  1. How I could possibly have coded this for flexibility to better anticipate future needs. Example: If I later needed to perform the same operations on a larger range of numbers.
  2. If I am using extraneous/obvious facts in my commenting or if my commenting is otherwise not in good practice.
  3. What more insight I might be able to gain in general regarding my current coding practices. Thank you.
share|improve this question

3 Answers 3

up vote 3 down vote accepted

If you're interested in more idiomatic Ruby, my advice would be to thoroughly understand Array, Enumerable, and the functional style enabled by using blocks.

The functional style allows you to use the output of one method directly as input to another method without intermediate variables. This is known as method chaining, and it reduces the number of intermediate variables you need to create.

Let's start with your array creation:

# Initializes array and iterator.
x = []
i = 0

# Stores 4000 random numbers 0..1 in array 'x'    
while i < 4000
  x << rand(2)
  i += 1
end

First, a bit about code comments. These are a bit gratuitous, as these repeat what the code says. Good comments should explain why something was done, not what is being done. If you feel that the code is non-obvious enough that you'll need a what comment, then it's time to think about refactoring so the code is a bit more self-explanatory.

I've been programming in Ruby for years and have never needed an iteration variable or a while loop. Why? Because the iteration methods such as Array's each or Enumerable's each_with_index are so powerful. So, as you mentioned, you could use times and do this:

x = []
4000.times do
  x << rand(2)
end

This is an improvement, but in this case we can do better. If you look at the constructor options for Array, you'll notice that you can specify the size and also pass a block for an initial value. Therefore, this can be written as:

Array.new(4000){ rand(2) }

#=> [1, 0, 0, 1, 0, 1, 0, 0, 1, 1, 1, 0, 1, 0...]

The output is the same: a 4000-element array consisting of random zeros and ones.

Now let's look at what you want to do next, according to your comment:

# Initializes array for the purpose of storing zeros and ones in separate
#  arrays for the sake of counting how many instances of each occur in the sample

So we want to separate the array into two buckets, based on their value. Thinking functionally, we should ask if, instead of iterating over intermediate output and creating a new data structure (which is also intermediate output because it will be thrown away once you count them), is there something we can do directly to the array? It turns out that the Enumerable module contains partition which can take a block that does exactly that.

Array.new(4000){ rand(2) }.partition{ |digit| digit.zero? }

#=> [[0, 0, 0, 0, 0, 0, 0, 0, ...], [1, 1, 1, 1, 1, 1, 1, 1, ...]]

Partition will separate the array into two sub-arrays: one for which the block returns true; and the other for which the block returns false. Note that I also used the zero? method which is available for numbers (see Fixnum#zero?) where I could have just said digit == 0. Either one is fine, but using zero? allows me to use a shortcut. This is equivalent:

Array.new(4000){ rand(2) }.partition(&:zero?)

#=> [[0, 0, 0, 0, 0, 0, 0, 0, ...], [1, 1, 1, 1, 1, 1, 1, 1, ...]]

This is known as the Symbol#to_proc trick which I won't go into detail here, but it basically allows you to shorten a block in the form {|x| x.method} to &:method. Whenever you want to call the same method on every item in an array, it is useful. You'll see this quite often in Ruby code these days.

Now, you don't really want these sub-arrays, you just want to know how many zeros and ones there are. Again thinking functionally, for each element in the array, you'd like its size. Enumerable's map is useful for transforming each element of an array.

Array.new(4000){ rand(2) }.partition(&:zero?).map{ |subarray| subarray.size }

#=> [2038, 1962]

Which, using the Symbol#to_proc trick can be shortened to

Array.new(4000){ rand(2) }.partition(&:zero?).map(&:size)

#=> [1994, 2006]

So there you have it: using idiomatic Ruby and functional style, you can reduce the first 28 lines down to a single short, yet readable line. To answer question #1, any speed difference would be insignificant with arrays of this size. (Thought it would be interesting to benchmark the two approaches with huge arrays)

share|improve this answer
    
How do you access the partitioned arrays to calculate the percentage? And say I wanted to work with a larger range of randomly generated numbers, like (1..64). Would partition become inappropriate? –  Bodhidarma Feb 1 '13 at 21:41
    
Partition only separates into two parts. But Enumerable has group_by which handles an arbitrary number of parts. I highly recommend becoming very familiar with the methods available to you on Array and Enumerable. –  Mark Thomas Feb 1 '13 at 22:36

It looks like you've done some procedural programming before? In Ruby you usually avoid using iterator variables like the i for simplicity.

The first while loop could be: 4000.times { x << rand(2) } A while loops is actually slightly quicker (>10%) but seldom used because object iterators are prettier.

You could also use a functional approach like: randoms = 4000.times.map { rand(2) }

Also, descriptive, variable names are generally preferable.

As for the second half of the script. Why store all the zeros and ones when you can just count them? This definitely will be faster as it doesn't require building up an array.

e.g. zeros_ratio = randoms.count(0) / randoms.length.to_f * 100

So if you study http://www.ruby-doc.org/core-1.9.3/Array.html a bit, you'll see how you can do a lot with 3 lines!

You could make the script more flexible of course by parameterising it. Set 4000 to a variable at the beginning instead of referencing it explicitly in the working bits of your code.

Commenting is nice, but usually a bit higher level than what you have there. Assume the reader could understand your code by reading it (so they can guess what a variable assignment to an appropriately named variable means), but comment to save them from having to think too hard about what the structure of your code is or what more complicated or crucial bits do.

One general piece of advice for learning coding is to read the code of more advanced programmers.

share|improve this answer

When you do not provide a block to a method from a container which is expecting one, Ruby (1.9) does generally not return an Error but an Enumerator, which has all kinds of methods of it's own:

puts 4000.times.count{ rand(2).zero? } #=> 1975

Choosing between times or each ((0..4000).each.count...) or for or while is not very important. It's what you do inside the loop that , ehm, counts.

share|improve this answer

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.