Skip to main content
Added generator suggestion
Source Link
200_success
  • 144.2k
  • 22
  • 188
  • 473

I'd prefer to see counting loops written as 1.upto(100) do … end.

Ruby case blocks are expressions. The puts can be factored out:

1.upto(100) do |n|
  i_3 = (n % 3 == 0)
  i_5 = (n % 5 == 0)
  puts case
    when i_3 && i_5
      'fizzbuzz'
    when i_3
      'fizz'
    when i_5
      'buzz'
    else
      n
  end
end

Personally, I'd go further: instead of treating i_3 and i_5 as booleans, assign them a noise.

1.upto(100) do |n|
  fizz = (n % 3 == 0) ? 'Fizz' : nil
  buzz = (n % 5 == 0) ? 'Buzz' : nil
  puts case
    when fizz || buzz
      "#{fizz}#{buzz}"
    else
      n
  end
end

Or, replace case with a ternary expression:

1.upto(100) do |n|
  fizz = (n % 3 == 0) ? 'Fizz' : nil
  buzz = (n % 5 == 0) ? 'Buzz' : nil
  puts (fizz || buzz) ? "#{fizz}#{buzz}" : n
end

To enhance code reusability, I suggest putting the code into a function, and yielding the results instead of printing them directly.

def fizzbuzz(max=100)
  1.upto(max) do |n|
    fizz = (n % 3 == 0) ? 'Fizz' : nil
    buzz = (n % 5 == 0) ? 'Buzz' : nil
    yield (fizz || buzz) ? "#{fizz}#{buzz}" : n
  end
end

fizzbuzz { |fb| puts fb }

Note the Ruby whitespace conventions: two spaces of indentation (you used four), and some space on each side of binary operators (you used none).

I'd prefer to see counting loops written as 1.upto(100) do … end.

Ruby case blocks are expressions. The puts can be factored out:

1.upto(100) do |n|
  i_3 = (n % 3 == 0)
  i_5 = (n % 5 == 0)
  puts case
    when i_3 && i_5
      'fizzbuzz'
    when i_3
      'fizz'
    when i_5
      'buzz'
    else
      n
  end
end

Personally, I'd go further: instead of treating i_3 and i_5 as booleans, assign them a noise.

1.upto(100) do |n|
  fizz = (n % 3 == 0) ? 'Fizz' : nil
  buzz = (n % 5 == 0) ? 'Buzz' : nil
  puts case
    when fizz || buzz
      "#{fizz}#{buzz}"
    else
      n
  end
end

Or, replace case with a ternary expression:

1.upto(100) do |n|
  fizz = (n % 3 == 0) ? 'Fizz' : nil
  buzz = (n % 5 == 0) ? 'Buzz' : nil
  puts (fizz || buzz) ? "#{fizz}#{buzz}" : n
end

Note the Ruby whitespace conventions: two spaces of indentation (you used four), and some space on each side of binary operators (you used none).

I'd prefer to see counting loops written as 1.upto(100) do … end.

Ruby case blocks are expressions. The puts can be factored out:

1.upto(100) do |n|
  i_3 = (n % 3 == 0)
  i_5 = (n % 5 == 0)
  puts case
    when i_3 && i_5
      'fizzbuzz'
    when i_3
      'fizz'
    when i_5
      'buzz'
    else
      n
  end
end

Personally, I'd go further: instead of treating i_3 and i_5 as booleans, assign them a noise.

1.upto(100) do |n|
  fizz = (n % 3 == 0) ? 'Fizz' : nil
  buzz = (n % 5 == 0) ? 'Buzz' : nil
  puts case
    when fizz || buzz
      "#{fizz}#{buzz}"
    else
      n
  end
end

Or, replace case with a ternary expression:

1.upto(100) do |n|
  fizz = (n % 3 == 0) ? 'Fizz' : nil
  buzz = (n % 5 == 0) ? 'Buzz' : nil
  puts (fizz || buzz) ? "#{fizz}#{buzz}" : n
end

To enhance code reusability, I suggest putting the code into a function, and yielding the results instead of printing them directly.

def fizzbuzz(max=100)
  1.upto(max) do |n|
    fizz = (n % 3 == 0) ? 'Fizz' : nil
    buzz = (n % 5 == 0) ? 'Buzz' : nil
    yield (fizz || buzz) ? "#{fizz}#{buzz}" : n
  end
end

fizzbuzz { |fb| puts fb }

Note the Ruby whitespace conventions: two spaces of indentation (you used four), and some space on each side of binary operators (you used none).

Source Link
200_success
  • 144.2k
  • 22
  • 188
  • 473

I'd prefer to see counting loops written as 1.upto(100) do … end.

Ruby case blocks are expressions. The puts can be factored out:

1.upto(100) do |n|
  i_3 = (n % 3 == 0)
  i_5 = (n % 5 == 0)
  puts case
    when i_3 && i_5
      'fizzbuzz'
    when i_3
      'fizz'
    when i_5
      'buzz'
    else
      n
  end
end

Personally, I'd go further: instead of treating i_3 and i_5 as booleans, assign them a noise.

1.upto(100) do |n|
  fizz = (n % 3 == 0) ? 'Fizz' : nil
  buzz = (n % 5 == 0) ? 'Buzz' : nil
  puts case
    when fizz || buzz
      "#{fizz}#{buzz}"
    else
      n
  end
end

Or, replace case with a ternary expression:

1.upto(100) do |n|
  fizz = (n % 3 == 0) ? 'Fizz' : nil
  buzz = (n % 5 == 0) ? 'Buzz' : nil
  puts (fizz || buzz) ? "#{fizz}#{buzz}" : n
end

Note the Ruby whitespace conventions: two spaces of indentation (you used four), and some space on each side of binary operators (you used none).