10
\$\begingroup\$

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?

asked Aug 4, 2014 at 5:12
\$\endgroup\$
8
  • 1
    \$\begingroup\$ I like your idea of passing test classes into the constructor for FizzBuzzer. I recommend having ModuloTester and SquareRootTester inherit from one class, and using Enumerable methods like select and map for the run method. \$\endgroup\$ Commented Aug 4, 2014 at 5:27
  • \$\begingroup\$ Also, your naming scheme looks fine. \$\endgroup\$ Commented Aug 4, 2014 at 5:31
  • 4
    \$\begingroup\$ @NicolasMcCurdy Could you please write answers as answers, and not as comments? For more information see "your code looks correct answers" \$\endgroup\$ Commented Aug 4, 2014 at 6:45
  • \$\begingroup\$ @NicolasMcCurdy I thought about using an inheritance hierarchy, but I keep hearing about the power of duck typing in Ruby, so I just decided to implement the same methods. How common is using inheritance in Ruby code versus just duck typing? \$\endgroup\$ Commented Aug 4, 2014 at 15:53
  • \$\begingroup\$ @cbojar Inheritance and duck-typing aren't mutually exclusive, though. Both are used in Ruby. So is composing classes with modules (such as Enumerable in 200_success' answer). Duck-typing is basically what you'd use an interface or protocol for in other OOP languages, and inheritance is regular inheritance (e.g. Car inheriting from Vehicle). In your code, there isn't much to inherit though, so duck-typing is a good solution. Just think of duck-typing as you implementing an interface, and inheritance as regular inheritance. \$\endgroup\$ Commented Aug 4, 2014 at 23:46

1 Answer 1

8
\$\begingroup\$

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 }
answered Aug 4, 2014 at 6:50
\$\endgroup\$
3
  • \$\begingroup\$ Thanks for your answer! I like the use of Enumerable, +1 on that! As to the two methods, I felt like it might be a tell-don't-ask situation, but I figured I'd see how people responded. +1 there too! As to the parens, I usually work in those "parens everywhere!" languages (and so also find it more readable), but I've heard that spurious parens are not very Ruby. I guess my concerns were overblown there. Lastly, quick question. In the def of FizzBuzz#each, you use values=nil, then set the default below. Is there a reason not to do values = 1..Float::INFINITY? \$\endgroup\$ Commented Aug 4, 2014 at 16:09
  • \$\begingroup\$ See Rev 2. I've corrected the handling of the default argument for #each, and explained the recommendation for parentheses. \$\endgroup\$ Commented Aug 4, 2014 at 17:05
  • \$\begingroup\$ This is an excellent code review. I appreciate the efforts you put in, and will note the tips you have provided. Thank you. Answer accepted! \$\endgroup\$ Commented Aug 5, 2014 at 3:02

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.