FizzBuzz in Ruby

Question

I have this implementation of the FizzBuzz challenge in Ruby:

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

It prints the numbers and words just as I would expect it to.

Is there a way to make this better follow Ruby best practices?

Answer 1

• You've inlined the whole program. It would be beneficial to separate FizzBuzz into it's own method that accepts n as an argument.
• Speaking of n, I discourage the use of one and two letter variable names. number would be better.
• I like that you precalculated booleans before entering the select statement, but these two need better names. divisibleBy3 and divisibleBy5 would be better.
• I also like that you interpreted the requirements correctly and are not concatenating the results in an attempt to prematurely optimize. See this for more info.
• Some of your statements could use some breathing space. (n % 3) instead of (n%3) etc.
• Brackets aren't necessary in Ruby, but they help to clarify.
• As @rofl pointed out in chat, you're printing "fizz", "buzz", and "fizzbuzz" instead of "Fizz", "Buzz", and "FizzBuzz".

Implementing my suggestions, you get something like this.

def fizzbuzz(number)
    divisibleBy3 = (number % 3 == 0)
    divisibleBy5 = (number % 5 == 0)

    case
        when divisibleBy3 && divisibleBy5
            puts "FizzBuzz"
        when divisibleBy3
            puts "Fizz"
        when divisibleBy5
            puts "Buzz"
        else 
            puts number
    end
end

(1..100).each {|n| fizzbuzz n}

Answer 2

As ckuhn203 pointed out, the variable names aren't great. In this case, I'd consider just calling them fizz and buzz.

Otherwise it's OK. There are so many different ways to do this. The case statement is a good choice for the usage here, but you could also do:

(1..100).each do |number|
  fizz = number % 3 == 0
  buzz = number % 5 == 0
  print "Fizz" if fizz
  print "Buzz" if buzz
  print number if !fizz && !buzz
  print "\n"
end

You could also use print number unless fizz || buzz but using unless with compound conditions can quickly become confusing to read, so I'd rather use good ol' if when anything more than a single boolean expression is involved.

Or use string concatenation

(1..100).each do |number|
  line = ""
  line << "Fizz" if number % 3 == 0
  line << "Buzz" if number % 5 == 0
  puts line.empty? ? number : line
end

Or, if you want a more flexible approach, you could do something like

denominators = { "Fizz" => 3, "Buzz" => 5 } # or more

(1..100).each do |number|
  matches = denominators.map { |name, divisor| name if number % divisor == 0 }
  puts matches.any? ? matches.join : number
end

Note that hashes are unordered in Ruby 1.8 and below, so it won't necessarily work correctly there, possibly printing "BuzzFizz". However, you can just use nested arrays instead to ensure ordering: [["Fizz", 3], ["Buzz", 5]]

And of course, any of these could be wrapped as methods, as ckuhn suggested.

Update: So apparently (see comments) the FizzBuzz task can be construed as printing "Fizz", "Buzz" or "FizzBuzz" as separate, distinct strings without using concatenation. The original problem statement does not, to my eyes, state this. It simply gives the expected output. From there, it's up to you (which is the point of the task, really)

Still, if the actual point is to print 3 distinct strings, then you can do something like

denominators = { "Whatever" => 15, "Fizz" => 3, "Buzz" => 5 }

# since ordering matters, you could just sort the hash (making it an array in process) to
# have the highest denominators first, like so:
# 
#   denominators.sort_by(&:last).reverse

(1..100).each do |number|
  match = denominators.detect { |name, divisor| number % divisor == 0 }
  puts match ? match.first : number
end

But again, I'd argue that the original spec does not require any such thing.

---

This is just for fun, because Ruby lets you monkey-patch anything. Of course you should not monkey-patch stuff like this "in real-life" - it's a super obnoxious "solution" I've just included for fun.

class Fixnum
  alias_method :original_to_s, :to_s

  def to_s
    str = ""
    str << "Fizz" if self % 3 == 0
    str << "Buzz" if self % 5 == 0
    str.empty? ? original_to_s : str
  end
end

puts (1..100).to_a # to_s gets called automatically

You just can't print integers normally anymore if you do this :-P

Answer 3

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).

Answer 4

I cannot add anything new to the comments of other users regarding your code. But striving for the most declarative code I can think of, I'd write:

class Integer
  def divisible_by?(n)
    (self % n).zero?
  end
end

1.upto(100) do |n| 
  string = case
    when n.divisible_by?(3) && n.divisible_by?(5) then 'fizzbuzz'
    when n.divisible_by?(3) then 'fizz'
    when n.divisible_by?(5) then 'buzz'
    else n.to_s
  end

  puts(string)
end

(Well, actually I think strings = 1.upto(100).lazy.map { ... } + print_lines(strings) would be still more declarative, but this style is not -yet- idiomatic in Ruby).

Answer 5

Here's how I would implement this in ruby, not as a golfing or obscure route, but rather making use of a lot of best practice ruby idioms:

class FizzBuzz
  def initialize(start = 1, last = 100)
    @start = start
    @last = last
  end

  def each
    range.each do |n|
      yield fizzbuzz(n)
    end
  end

  private
  attr_reader :start, :last

  def range
    start..last
  end

  def divisible_by_3(n)
    n % 3 == 0
  end

  def divisible_by_5(n)
    n % 5 == 0
  end

  def divisible_by_15(n)
    n % 15 == 0
  end

  def fizzbuzz(n)
    case
      when divisible_by_15(n)
        'fizzbuzz'
      when divisible_by_3(n)
        'fizz'
      when divisible_by_5(n)
        'buzz'
      else
        n
    end    
  end
end

FizzBuzz.new(1,50).each {|f|  puts f}

What's idiomatic here?

• Start with a class. Default values in initializer yields the most basic example.
• Small public API. The rest of the methods are private, to indicate they aren't expected to be used directly.
• private attr_reader reads better than using instance variables directly
• separating the responsibility of output vs the generation of the data. The FizzBuzz class simply iterate over the range and returns the results. The user of the class is responsible for output, hence the each block at the bottom.
• small methods. Each method does one thing, and is named in such a way that you can easily tell what it does. reading the short method is easy to confirm what the method does.
• note that divisible by 3 and divisible by 5 implies divisible by 15. One could make an argument for making that method just use the previous methods: divisible_by_3 && divisible_by_5 but I think think dividing by 15 is better.

Yes, this is significantly longer than your example... But on larger projects with more complicated logic, following these idioms will greatly enhance the readability and thus, maintenance, of your code base.

Another version of the fizzbuzz method could be:

  def fizzbuzz(n)
    divisible_by_15(n) and return 'fizzbuzz'
    divisible_by_3(n)  and return 'fizz'
    divisible_by_5(n)  and return 'buzz'
    n
  end

This eliminates a supposed code smell of the case statement, but it's debatable whether it's better or not. Some people object to more than one return statement; others object to case statements... I could go either way.