3
\$\begingroup\$

The idea is to create a function that gets a number passed in and ouputs a multiplication grid.

For example the desired output for grid(10) would be :

1,2,3,4,5,6,7,8,9,10
2,4,6,8,10,12,14,16,18,20
3,6,9,12,15,18,21,24,27,30
4,8,12,16,20,24,28,32,36,40
5,10,15,20,25,30,35,40,45,50
6,12,18,24,30,36,42,48,54,60
7,14,21,28,35,42,49,56,63,70
8,16,24,32,40,48,56,64,72,80
9,18,27,36,45,54,63,72,81,90
10,20,30,40,50,60,70,80,90,100

I've come up with two variations of the same solution:


Variation 1

def grid1(grid_size)
  output = String.new
  for y in 1..grid_size
    line = Array.new
    for x in 1..grid_size
      line << x * y
    end
    output += "#{line.join(",")}\n"
  end

  output
end

Variation 2

def grid(grid_size)
  output = (1..grid_size).to_a.map do |y|
    (1..grid_size).to_a.map{|x| x*y}.join(",")
  end

  output
end

Any other ideas on how to solve this or ways that the current solutions could be improved would be most appreciated!

\$\endgroup\$
2
\$\begingroup\$

The second variant is definitely more expressive. If you can write the whole function as one expression, and that expression is readable, then do it that way.

There is no need to call Range#to_a before doing #map.

Naming could be better. grid_size is redundant when the function is already called grid. I'd also suggest row and col instead of y and x.

def grid(size)
  (1..size).map do |row|
    (1..size).map { |col| row * col }.join(',')
  end.join("\n")
end
\$\endgroup\$
3
\$\begingroup\$

return from function anti-pattern

For the second example, doing

x = computation()
x

In a function is an anti-pattern, you should return the result directly:

computation()

It is more common and easier for the same result.

No Array

The first function does not need an array, just append to the string directly.

\$\endgroup\$
3
\$\begingroup\$

Variation #2 is definitely the more Ruby-esque, in terms of structure. Plain for-loops rarely have a use in Ruby. But as Caridorc said, don't assign to a variable, only to return said variable: Return directly instead.

In any case, the task here is to transform a set of numbers into another set of numbers (printing it is yet another transformation). But what you want, in a sense, is a function \$f(x)\$ that just is a transformation. And that's what map does.

The first variation of course gives the same result in the end, but it's less about transforming something, and more like building a wholly new one (by pushing to an array).

But there's a 3rd option, and it's probably more expressive.

sequence = (1..grid_size).to_a
sequence
  .product(sequence)
  .map { |a, b| a * b }
  .each_slice(grid_size) { |row| puts row.join(",") }

The idea of using Array#product was taken from this question, which was posted a couple of days ago (literally) by Caridorc (again).

\$\endgroup\$
  • 1
    \$\begingroup\$ It is multiplication grid time :) product is very appropriate for this case. It was made to replace nested loops. \$\endgroup\$ – Caridorc Dec 1 '15 at 19:07

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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