Making code more functionally readable

Question

I am looking at following snippet. When map-getOrElse and nested patten matching increases in the code it doesn't look so elegant. What better options do you suggest?

 case MyMessage =>
        val image = (request \ "image").asOpt[String]
        image.map { im =>
          val conf = (request \ "confirmation").asOpt[String]
          conf.map { cf =>
            //code to retrieve ride

            ride match {
              case Some(r) =>
                if (booleanCondition) sender ! SuccessCommand(JsBoolean(true), command)
                else sender ! FailureCommand("Problem updating", command)
              case None => sender ! FailureCommand("Ride empty", command)
            }

          } getOrElse (sender ! FailureCommand("Missing number", command))

        } getOrElse (sender ! FailureCommand("Missing image", command))

You have `sender ! FailureCommand` four times. Maybe `def reportFailure(msg: String) = sender ! FailureCommand(msg, command)` at an appropriate spot (i.e. where `sender` and `command` are in scope)? — Rex Kerr, Apr 01 '14 at 21:03

score 2 · Answer 1 · edited May 23 '17 at 12:11

2

Whenever you are mapping over an Option with a function that produces an Option, you should consider whether you should be using flatMap:

def f(x: Int): Option[Int] = Some(x + 1)

f(1).flatMap(x => f(x)).flatMap(y => f(y))               // Some(4)
f(1).flatMap(x => f(x)).flatMap(y => f(y)).getOrElse(0)  // 4

You can also use for-comprehensions for this, which is really nice for having clean code when you have long chains of these:

(for(x <- f(1); y <- f(x); z <- f(y)) yield z).getOrElse(0)

edited May 23 '17 at 12:11

Community

1
1

answered Apr 01 '14 at 17:59

dhg

52,383
8
123
144

Thanks @dhg. I wonder how to represent original snippet from the question in terms of for comprehension especially those sender ! MEssage calls, which are different for each failure/getOrElse cases? – user2066049 Apr 01 '14 at 18:18

DNA · Answer 2 · 2014-04-07T11:01:52.750

Another way to tackle this is to return Either[Command,String] from various helper functions, rather than Option. This would then allow you to use a for comprehension, something like the following:

val result = for {
  i <- getImage().right
  c <- getConf().right
  r <- getRide().right
  z <- check(r).right
} yield z

// extract either left or right, whichever is occupied
sender ! result.fold(identity, _ => success())

This has the desired property that we stop as soon as we encounter an error, and capture that specific error - or proceed to a successful conclusion.

DNA · Answer 3 · 2014-04-01T21:52:46.003

I think you should be able to collapse a lot of this into Option.fold(), roughly as follows:

case MyMessage =>
  sender ! 
    getImage().fold(fail("Missing image")) { im =>
      getConf().fold(fail("Missing number")) { conf => // conf isn't used
        getRide().fold(fail("Ride empty")) { r =>
          if (booleanCondition) succeed(true)
          else fail("Problem updating")
        }
      }
    }

This turns out a bit more concise than flatMap and orElse in this situation (see below)

Option.fold(ifEmpty){f} returns ifEmpty (evaluated lazily) if the option was empty, or evaluates the function f if the option was full.

The above code assumes you create helper functions for getting the various Options (or you could inline the relevant code). It also assumes you pull out the creation of commands into a helper function or two, to avoid all the duplicate references to command.

For comparison, a solution using flatMap looks something like:

case MyMessage =>
  sender ! 
    getImage().flatMap { im =>
      getConf().flatMap { conf =>
        getRide().flatMap { r =>
          if (booleanCondition) Some(succeed(true))
          else Some(fail("Problem updating"))
        }.orElse(Some(fail("Ride Empty")))
      }.orElse(Some(fail("Missing number")))
    }.getOrElse(fail("Missing image"))

which you could shorten very slightly by having variants of your helper methods (fail and succeed) that return Some[Command] rather than Command

Making code more functionally readable

3 Answers3