Skip to main content
replaced http://codereview.stackexchange.com/ with https://codereview.stackexchange.com/
Source Link

As ckuhn203 pointed outAs ckuhn203 pointed out, the variable names aren't great. In this case, I'd consider just calling them fizz and buzz.

As ckuhn203 pointed out, the variable names aren't great. In this case, I'd consider just calling them fizz and buzz.

As ckuhn203 pointed out, the variable names aren't great. In this case, I'd consider just calling them fizz and buzz.

added 1021 characters in body
Source Link
Flambino
  • 33k
  • 2
  • 45
  • 90

Update: So apparently (see comments) the FizzBuzz task can be construed as printing "Fizz", "Buzz" or "FizzBuzz" as separate, distinct strings without using concatenation. The original problem statement does not, to my eyes, state this. It simply gives the expected output. From there, it's up to you (which is the point of the task, really)

Still, if the actual point is to print 3 distinct strings, then you can do something like

denominators = { "Whatever" => 15, "Fizz" => 3, "Buzz" => 5 }

# since ordering matters, you could just sort the hash (making it an array in process) to
# have the highest denominators first, like so:
# 
#   denominators.sort_by(&:last).reverse

(1..100).each do |number|
  match = denominators.detect { |name, divisor| number % divisor == 0 }
  puts match ? match.first : number
end

But again, I'd argue that the original spec does not require any such thing.

Update: So apparently (see comments) the FizzBuzz task can be construed as printing "Fizz", "Buzz" or "FizzBuzz" as separate, distinct strings without using concatenation. The original problem statement does not, to my eyes, state this. It simply gives the expected output. From there, it's up to you (which is the point of the task, really)

Still, if the actual point is to print 3 distinct strings, then you can do something like

denominators = { "Whatever" => 15, "Fizz" => 3, "Buzz" => 5 }

# since ordering matters, you could just sort the hash (making it an array in process) to
# have the highest denominators first, like so:
# 
#   denominators.sort_by(&:last).reverse

(1..100).each do |number|
  match = denominators.detect { |name, divisor| number % divisor == 0 }
  puts match ? match.first : number
end

But again, I'd argue that the original spec does not require any such thing.

added 230 characters in body
Source Link
Flambino
  • 33k
  • 2
  • 45
  • 90

As ckuhn203 pointed out, the variable names aren't great. In this case, I'd consider just calling them fizz and buzz.

Otherwise it's OK. There are so many different ways to do this. The case statement is a good choice for the usage here, but you could also do:

(1..100).each do |i||number|
  fizz = inumber % 3 == 0
  buzz = inumber % 5 == 0
  print "Fizz" if fizz
  print "Buzz" if buzz
  print inumber if !fizz && !buzz
  print "\n"
end

You could also use print number unless fizz || buzz but using unless with compound conditions can quickly become confusing to read, so I'd rather use good ol' if when anything more than a single boolean expression is involved.

Or use string concatenation

(1..100).each do |number|
  line = ""
  line << "Fizz" if number % 3 == 0
  line << "Buzz" if number % 5 == 0
  puts line.empty? ? number : line
end

Or, if you want a more flexible approach, you could do something like

denominators = { "Fizz" => 3, "Buzz" => 5 } # or more

(1..100).each do |number|
  matches = denominators.map { |name, divisor| name if number % divisor == 0 }
  puts matches.any? ? matches.join : number
end

Note that hashes are unordered in Ruby 1.8 and below, so it won't necessarily work correctly there, possibly printing "BuzzFizz". However, you can just use nested arrays instead to ensure ordering: [["Fizz", 3], ["Buzz", 5]]

And of course, any of these could be wrapped as methods, as ckuhn suggested.


This is just for fun, because Ruby lets you monkey-patch anything. Of course you should not monkey-patch stuff like this "in real-life" - it's a super obnoxious "solution" I've just included for fun.

class Fixnum
  alias_method :original_to_s, :to_s

  def to_s
    str = ""
    str << "Fizz" if self % 3 == 0
    str << "Buzz" if self % 5 == 0
    str.empty? ? original_to_s : str
  end
end

puts (1..100).to_a # to_s gets called automatically

You just can't print integers normally anymore if you do this :-P

As ckuhn203 pointed out, the variable names aren't great. In this case, I'd consider just calling them fizz and buzz.

Otherwise it's OK. There are so many different ways to do this. The case statement is a good choice for the usage here, but you could also do:

(1..100).each do |i|
  fizz = i % 3 == 0
  buzz = i % 5 == 0
  print "Fizz" if fizz
  print "Buzz" if buzz
  print i if !fizz && !buzz
  print "\n"
end

Or use string concatenation

(1..100).each do |number|
  line = ""
  line << "Fizz" if number % 3 == 0
  line << "Buzz" if number % 5 == 0
  puts line.empty? ? number : line
end

Or, if you want a more flexible approach, you could do something like

denominators = { "Fizz" => 3, "Buzz" => 5 } # or more

(1..100).each do |number|
  matches = denominators.map { |name, divisor| name if number % divisor == 0 }
  puts matches.any? ? matches.join : number
end

And of course, any of these could be wrapped as methods, as ckuhn suggested.


This is just for fun, because Ruby lets you monkey-patch anything. Of course you should not monkey-patch stuff like this "in real-life" - it's a super obnoxious "solution" I've just included for fun.

class Fixnum
  alias_method :original_to_s, :to_s

  def to_s
    str = ""
    str << "Fizz" if self % 3 == 0
    str << "Buzz" if self % 5 == 0
    str.empty? ? original_to_s : str
  end
end

puts (1..100).to_a # to_s gets called automatically

You just can't print integers normally anymore if you do this :-P

As ckuhn203 pointed out, the variable names aren't great. In this case, I'd consider just calling them fizz and buzz.

Otherwise it's OK. There are so many different ways to do this. The case statement is a good choice for the usage here, but you could also do:

(1..100).each do |number|
  fizz = number % 3 == 0
  buzz = number % 5 == 0
  print "Fizz" if fizz
  print "Buzz" if buzz
  print number if !fizz && !buzz
  print "\n"
end

You could also use print number unless fizz || buzz but using unless with compound conditions can quickly become confusing to read, so I'd rather use good ol' if when anything more than a single boolean expression is involved.

Or use string concatenation

(1..100).each do |number|
  line = ""
  line << "Fizz" if number % 3 == 0
  line << "Buzz" if number % 5 == 0
  puts line.empty? ? number : line
end

Or, if you want a more flexible approach, you could do something like

denominators = { "Fizz" => 3, "Buzz" => 5 } # or more

(1..100).each do |number|
  matches = denominators.map { |name, divisor| name if number % divisor == 0 }
  puts matches.any? ? matches.join : number
end

Note that hashes are unordered in Ruby 1.8 and below, so it won't necessarily work correctly there, possibly printing "BuzzFizz". However, you can just use nested arrays instead to ensure ordering: [["Fizz", 3], ["Buzz", 5]]

And of course, any of these could be wrapped as methods, as ckuhn suggested.


This is just for fun, because Ruby lets you monkey-patch anything. Of course you should not monkey-patch stuff like this "in real-life" - it's a super obnoxious "solution" I've just included for fun.

class Fixnum
  alias_method :original_to_s, :to_s

  def to_s
    str = ""
    str << "Fizz" if self % 3 == 0
    str << "Buzz" if self % 5 == 0
    str.empty? ? original_to_s : str
  end
end

puts (1..100).to_a # to_s gets called automatically

You just can't print integers normally anymore if you do this :-P

Source Link
Flambino
  • 33k
  • 2
  • 45
  • 90
Loading