Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

Would love some feedback on this. I'm coming from a Java background and kinda feel like I've just done exactly what I would do in Java. Is there a better way?

for (i <- 1 to 100) {
  if ( i % 3 == 0 && i% 5 == 0) 
    println(i +" = FizzBuzz")
  else if (i % 3 == 0) 
    println(i +" = Fizz")
  else if (i % 5 == 0) 
    println(i +" = Buzz")
}
share|improve this question

5 Answers 5

up vote 19 down vote accepted

FizzBuzz is kind of a difficult example because its simplicity means it looks more or less the same in every language. That said, we can go out of our way to emphasize the functional aspect of Scala.

First, we can wrap the core logic of FizzBuzz up in its own function:

def fizzBuzz(x:Int) = {
  if (x % 15 == 0)
    "FizzBuzz"
  else if (x % 3 == 0)
    "Fizz"
  else if (x % 5 == 0)
    "Buzz"
  else
    x
}

Now, we can compose this with printing and map it over the domain we're interested in:

(1 until 100).map(fizzBuzz _ andThen println)

Notice that, where in Java we would probably repeat the println in every case, in Scala, with first-class functions, we can easily separate it out as its own step in the process.

The advantage of this becomes clearer when we think about recombining elements. If we wanted to, we could, for example, join everything in a string without rewriting the essential FizzBuzzzing logic:

println((1 until 100).map(fizzBuzz).mkString(", "))

So, hopefully you can see that Scala's "better way" is first-class functions. With them, we more easily compose the pieces of our system.

share|improve this answer
1  
As a disclaimer, I'm not a Scala guy, so that code can probably be cleaned up a little. Still, the fundamental takeaway is the same. –  Beyamor Feb 2 at 9:59
    
Hey, that's cool - I like it. This is the kind of response I was looking for. It's really a bit of a shift in thinking, I think that a simple example like FizzBuzz makes it really clear. –  cm22 Feb 2 at 11:19
1  
@cookiemonster you might want to also glance at Higher-order FizzBuzz in Clojure. I think that calculating pi (#18 on 20 controversial programming opinions) because it isn't quite as simple (not hard, but not quite as simple) and really does require some different thinking in the functional world. –  MichaelT Feb 2 at 18:20

Or if you prefer match/case to if/else:

(1 until 100).map(i => (i % 3, i % 5) match {
  case (0, 0) => "FizzBuzz"
  case (0, _) => "Fizz"
  case (_, 0) => "Buzz"
  case _ => i
}).foreach(println)

Update:

So what we are doing here is taking list of numbers and mapping them first to tuples where on the left side is that number module 3 and on the right side modulo 5. Then we are matching those tuples to cases where both are zero, left is zero, right is zero or neither is zero.

Also the actual logic is in the map-block and side-effect of printing is in the foreach-block.

share|improve this answer
    
This is by far the most scala-y solution proposed yet, IMO. map() is preferred over standard for loops, and the match here is much more elegant than the chained if/else. –  geoffjentry Feb 2 at 16:24
  • Instead of using both 3 and 5:

    if (i % 3 == 0 && i % 5 == 0)
    

    you can just use 15:

    if (i % 15 == 0)
    
  • You're also supposed to print the number by itself if neither case applies:

    else
        println(i)
    

    For your existing cases, however, you're only supposed to print the message.

Final solution:

for (i <- 1 to 100) {
  if (i % 15 == 0) 
    println("FizzBuzz")
  else if (i % 3 == 0) 
    println("Fizz")
  else if (i % 5 == 0) 
    println("Buzz")
  else
    println(i)
}
share|improve this answer
    
And I thought you didn't know Scala ;) –  Simon André Forsberg Feb 2 at 9:04
2  
@SimonAndréForsberg: Shhhhh! :P –  Jamal Feb 2 at 9:14

I tried to come up with a different looking solution. I am not claiming it is better, but I thought I should put it here since this approach might be more appropriate for some applications which are somewhat related to FizzBuzz.

Basically I treat the fizz and buzz problems separately and merge them. I also use a lot of Scala features such as Stream, Option, currying and case matching.

  def transformPeriodic(period: Int, word: String)(counter: Int) =
    if (counter % period == 0) Some(word) else None

  val fizzTransform = transformPeriodic(3, "Fizz") _
  val buzzTransform = transformPeriodic(5, "Buzz") _

  def mergeFizzBuzz(fizzOpt: Option[String], buzzOpt: Option[String]): Option[String] =   {
    (fizzOpt, buzzOpt) match {
       case (None, None) => None
       case _ => Some(fizzOpt.getOrElse("") + buzzOpt.getOrElse(""))
    }
  }

  val streamInt = Stream.from(1)
  val tripletStream = streamInt.map(i => (i, fizzTransform(i), buzzTransform(i)))
  val doubletStream = tripletStream.map { case (i, fizzOpt, buzzOpt) => (i, mergeFizzBuzz(fizzOpt, buzzOpt)) }
  val fizzBuzzStream = doubletStream.map { case (i, fizzBuzzOpt) => fizzBuzzOpt.getOrElse(i) }

  fizzBuzzStream take 15 foreach println
share|improve this answer

A bit old but here's my two cents: you could use PartialFunction.

Basically you define the functions you want to use and compose them in the proper order:

def f(divisor: Int, result: Int => String): PartialFunction[Int, String] = {
  case i if (i % divisor == 0) => result(i)
}
val f3  = f(3,  _ => "Fizz")
val f5  = f(5,  _ => "Buzz")
val f15 = f(15, _ => "FizzBuzz")
val id  = f(1,  _.toString)

val fizzBuzz = f15 orElse f3 orElse f5 orElse id
(1 to 100).map(fizzBuzz).foreach(println)

f15 must be defined first otherwise f3 or f5 will stop the composition. In the same way id is the last fallback (any number is modulo 1) and print the input (that's why I defined result as a function, and not just a string).

share|improve this answer

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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