6
\$\begingroup\$

On my quest to learn Ruby I figured exploring the proper usage of different data-structures would be invaluable, including strings, arrays and hashes.

I decided to build a hash containing the numbers 1 till 100 inclusive. These hashes would contain:

  • Whether the number is prime
  • What it's FizzBuzz output would be
  • The n-th Fibonacci number.

In the end the number itself turned into a value as well. I intended the number to be the key and the other 3 values to be a hash of that key but it got too complicated.

As a side effect my project got quite bulky and should probably be split up. Somehow. My gut tells me I should move the FizzBuzz and Fibonacci parts to their own module and import them like I did with the already existing prime module.

On top of that, I got a couple of concerns:

  • Is output += "Fizz" the idiomatic way of adding a string to a string?
  • Is my usage of the hash proper or is it not supposed to be used like this?
  • Did I adhere to variable naming rules?
  • How is the general readability?

Code:

require 'prime'

class Fibonacci
    def initialize(max)
        @max = max
        @list = [1, 1, 2]
        for i in 3..@max do
            @list << @list[i-2] + @list[i-1]
        end
    end
    def get_nth(n)
        return @list[n]
    end
end

numbers = []

def fizzbuzz(number)
    output = ""
    if (number % 3 == 0)
        output += "Fizz"
    end
    if (number % 5 == 0)
        output += "Buzz"
    end
    if output == ""
        output = number
    end
    return output
end

Fib = Fibonacci.new(100)

for number in 1..100 do
    hash = {
        :Number => number,
        :FizzBuzz => fizzbuzz(number),
        :Prime => Prime.instance.prime?(number),
        :Fibonacci => Fib.get_nth(number)
    }
    numbers << hash
end

The results can be called like this:

Primes:

numbers.each { |number| puts number[:Number], number[:Prime] }

FizzBuzz:

numbers.each { |number| puts number[:FizzBuzz] }

Fibonacci:

numbers.each { |number| puts number[:Number], number[:Fibonacci] }

Try it

\$\endgroup\$
4
\$\begingroup\$

I'm not convinced that there is much value in combining the results of three separate problems into one data structure, but I'll play along.

Note that the standard indentation for Ruby is two spaces.

Fibonacci

I don't think it's a good idea to have Fibonacci objects that differ only in their max. What is the point of the max, anyway? It just introduces a possible failure if you try to request an entry beyond the max. Why not have a "smart" singleton that extends itself as necessary?

The get_nth method is a natural candidate for overloading the [] operator, so that you can write Fibonacci[3].

Opinions may differ as to the convention, but I would consider 0 to be the zeroth Fibonacci number, followed by 1, 1, 2, 3, 5, …

class Fibonacci
  @list = [0, 1]

  def self.[](nth)
    while @list.size <= nth
      @list << @list[-2] + @list[-1]
    end
    @list[nth]
  end
end

FizzBuzz

It would probably be a good idea to make FizzBuzz adhere to the same interface as Fibonacci.

Your results are sometimes numbers and sometimes strings. That's allowable, but not a practice that I would recommend.

The conditions can be written a bit more succinctly using the value if condition syntax and implicit return.

class FizzBuzz
  def self.[](n)
    output = ''
    output += 'Fizz' if n % 3 == 0
    output += 'Buzz' if n % 5 == 0
    output = n.to_s if output.empty?
    output
  end
end

numbers

Instead of appending to an empty array, build it "all at once" using Range#collect.

numbers = (1..100).collect do |n|
  {
    :Number => n,
    :FizzBuzz => FizzBuzz[n],
    :Prime => Prime.instance.prime?(n),
    :Fibonacci => Fibonacci[n]
  }
end
\$\endgroup\$
  • \$\begingroup\$ Turns out there are two versions of the Fibonacci sequence, a modern version starting at 0 and the original version starting at 1. \$\endgroup\$ – Mast Dec 11 '15 at 13:00
  • \$\begingroup\$ My IDE appears to have thought 4 was more appropriate, my IDE is no longer allowed an opinion. The FizzBuzz and hash look much better in your answer, that was definitely what I was looking for. At the risk of sounding stupid: won't the refactoring of Fibonacci cause it to be calculated again for each call? \$\endgroup\$ – Mast Dec 11 '15 at 13:16
  • \$\begingroup\$ As to why there are 3 separate problems in one data structure, well, I needed to figure out how the data structures worked and I needed diverse data. So I grabbed the three most obvious types of data I could think of when writing beginner code. \$\endgroup\$ – Mast Dec 11 '15 at 13:18
  • \$\begingroup\$ I suggest that you insert some puts calls in Fibonacci to find out when it calculates what. \$\endgroup\$ – 200_success Dec 11 '15 at 15:12
2
\$\begingroup\$

I can't comment yet, so I'll just respond here...

I completely agree with @200_success that putting all these different problems together is not a great idea. It seems to violate the Single Responsiblity Problem even before your starting.

Hash

The common way to write a hash with symbols is to use the name-colon-value not colon-name-arrow-value:

# Instead of this
{
  :Number => n,
  :FizzBuzz => FizzBuzz[n],
  :Prime => Prime.instance.prime?(n),
  :Fibonacci => Fibonacci[n]
}

# Try this:
{
  Number: n,
  FizzBuzz: FizzBuzz[n],
  Prime: Prime.instance.prime?(n),
  Fibonacci: Fibonacci[n]
}

Loops and Enumerators

Loops should generally be avoided in ruby. The one exception is maybe a while/until loop. A for loop should be avoided since it has different scoping rules from using an enumerator + block. Instead, try to use enumerators. They are shorter, simpler, safer (extra scope, usually hide loop counters, reduce mutable state, often remove the need for conditionals), and more explicit (e.g. what does this code do (1..10).select?(&:even) ).

Stlye Guide

The community style guide is an excellent guide. It doesn't just tell you what to do, but also why. And despite the name, it isn't merely about style either. You'll want to read it.

The style guide comes with a great gem that does a lot of checking automatically (but do read the guide): Rubocop

gem install rubocop
rubocop my_source_file.rb
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.