First off: If you want a range that goes from 1 to x, inclusive, write
Second, if it's the last line executed in a method, it's automatically returned -- so in
get_text, you don't need
return in the last line, just the actual code.
get_text should return the text to be printed, not what it might be. Move the empty check (
fb_text.empty ? i : fb_text) to
get_text instead of having it at the
puts call. See the code at the end for clarification.
if statements with just one line should be expressions at the end. Again, see the modified code for what I mean.
Methods defined in a class are generally talking about the object they're run on. I'd pull
is_number_divisible_by? out of the class and dropping it at the beginning of your code instead, and renaming it to something like
divisible_by? instead (since it's shorter and carries the same information).
Aside from that (and an odd extra newline at the beginning of your class definition), it looks good! Here's your code with these suggestions:
def divisible_by?(number, divisor)
number % divisor == 0
def initialize(fizz = 'Fizz', buzz = 'Buzz')
@fizz = fizz
@buzz = buzz
fizz_buzz_text = ''
fizz_buzz_text += @fizz if divisible_by? number, 3
fizz_buzz_text += @buzz if divisible_by? number, 5
fizz_buzz_text.empty? ? number : fizz_buzz_text
fb = FizzBuzz.new
(1..100).each do |i|
puts fb.get_text i
Note: Personally, I wouldn't do this in an OOP way. Ruby is much more oriented towards functional programming, though of course OOP is perfectly valid; besides, functional is a bit simpler. My preferred way to do it would be something like this:
(1..100).each do |num|
output = ''
output += 'Fizz' if num % 3 == 0
output += 'Buzz' if num % 5 == 0
puts output.empty? ? num : output