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

FizzBuzz in Ruby

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

Problem

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?

Solution


  • 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}

Code Snippets

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}

Context

StackExchange Code Review Q#56927, answer score: 16

Revisions (0)

No revisions yet.