HiveBrain v1.2.0
Get Started
← Back to all entries
patternrubyMinor

Object-Oriented FizzBuzz in Ruby

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
orientedobjectrubyfizzbuzz

Problem

I don't use Ruby on a regular basis, but I felt inspired to write an extensible, object-oriented version of FizzBuzz to sharpen my skills. I welcome all feedback.

The main class:

class FizzBuzzer
  def initialize(*tests)
    @tests = tests
  end

  def run(values)
    result = []
    values.each do |value|
      str = ''
      @tests.each do |test|
        str << test.word if test.satisfied_by? value
      end
      str = value.to_s if str.empty?
      result << "#{str}\n"
    end
    result.join
  end
end


Sample test operation classes:

class ModuloTester
  attr_accessor :word

  def initialize(modulo, word)
    @modulo = modulo.to_i
    @word = word
  end

  def satisfied_by?(number)
    (number % @modulo).zero?
  end
end

class SquareRootTester
  attr_accessor :word

  def initialize(word)
    @word = word
  end

  def satisfied_by?(number)
    (Math.sqrt(number) % 1).zero?
  end
end


In use:

fizz_buzzer = FizzBuzzer.new ModuloTester.new(3, 'Fizz'), ModuloTester.new(5, 'Buzz')
puts fizz_buzzer.run 1..100

puts

fizz_buzzer = FizzBuzzer.new SquareRootTester.new('(PerfectSquare)')
puts fizz_buzzer.run 1..10
puts fizz_buzzer.run 20..30
puts fizz_buzzer.run 45..55


Which produces the expected output. Is this idiomatic Ruby? Is the naming scheme acceptable? What can be improved?

Solution

I'd like to suggest two changes to your interface.

  • Implement Enumerable. Your object accepts an Enumerable; it would be nice if it also acted as an Enumerable. All you have to do is include Enumerable and rename runeach, and then it behaves more like the way a Rubyist would like to see.



  • Remove the separate calls to #satisfied_by? and #word. I find it awkward that FizzBuzzer should have to know about the workings of the tester objects like that. Why not just have the tester return a truthy output value to indicate that it has performed a transformation?



In addition, I would prefer to see the invocations of FizzBuzzer.new and fizz_buzzer.run written with parentheses, for readability. There appears to be a consensus among multiple Ruby style guides that parentheses should only be omitted for simpler or specialized calls such as puts, include, attr_accessor, or methods that take no arguments.

class FizzBuzz
  include Enumerable

  def initialize(*transforms)
    @transforms = transforms
  end

  def each(values=1..Float::INFINITY)
    values.each do |value|
      words = @transforms.map { |transform| transform.transform(value) }
      yield words.any? ? words.join('') : value.to_s
    end
  end
end

class ModuloTransform
  def initialize(modulo, word)
    @modulo = modulo.to_i
    @word = word
  end

  def transform(number)
    (number % @modulo).zero? ? @word : nil
  end
end

class SquareRootTransform
  def initialize(word)
    @word = word
  end

  def transform(number)
    (Math.sqrt(number) % 1).zero? ? @word : nil
  end
end

fb = FizzBuzz.new(ModuloTransform.new(3, 'Fizz'),
                  ModuloTransform.new(5, 'Buzz'))
# FizzBuzz naturally works from 1 to infinity.  This is one way to limit
# the results to the first 100.
fb.take(100).each { |word| puts word }

fb = FizzBuzz.new(SquareRootTransform.new('(PerfectSquare)'))
# This is another way to do FizzBuzz only for certain values.
fb.each(1..100) { |word| puts word }

Code Snippets

class FizzBuzz
  include Enumerable

  def initialize(*transforms)
    @transforms = transforms
  end

  def each(values=1..Float::INFINITY)
    values.each do |value|
      words = @transforms.map { |transform| transform.transform(value) }
      yield words.any? ? words.join('') : value.to_s
    end
  end
end

class ModuloTransform
  def initialize(modulo, word)
    @modulo = modulo.to_i
    @word = word
  end

  def transform(number)
    (number % @modulo).zero? ? @word : nil
  end
end

class SquareRootTransform
  def initialize(word)
    @word = word
  end

  def transform(number)
    (Math.sqrt(number) % 1).zero? ? @word : nil
  end
end

fb = FizzBuzz.new(ModuloTransform.new(3, 'Fizz'),
                  ModuloTransform.new(5, 'Buzz'))
# FizzBuzz naturally works from 1 to infinity.  This is one way to limit
# the results to the first 100.
fb.take(100).each { |word| puts word }

fb = FizzBuzz.new(SquareRootTransform.new('(PerfectSquare)'))
# This is another way to do FizzBuzz only for certain values.
fb.each(1..100) { |word| puts word }

Context

StackExchange Code Review Q#58981, answer score: 8

Revisions (0)

No revisions yet.