I have two functions which return Futures. I'm trying to feed a modified result from first function into the other using a for-yield comprehension.
This approach works:
val schoolFuture = for {
ud <- userStore.getUserDetails(user.userId)
sid = ud.right.toOption.flatMap(_.schoolId)
s <- schoolStore.getSchool(sid.get) if sid.isDefined
} yield s
However I'm not happy with having the "if" in there, it seems that I should be able to use a map instead.
But when I try with a map:
val schoolFuture: Future[Option[School]] = for {
ud <- userStore.getUserDetails(user.userId)
sid = ud.right.toOption.flatMap(_.schoolId)
s <- sid.map(schoolStore.getSchool(_))
} yield s
I get a compile error:
[error] found : Option[scala.concurrent.Future[Option[School]]]
[error] required: scala.concurrent.Future[Option[School]]
[error] s <- sid.map(schoolStore.getSchool(_))
I've played around with a few variations, but haven't found anything attractive that works. Can anyone suggest a nicer comprehension and/or explain what's wrong with my 2nd example?
Here is a minimal but complete runnable example with Scala 2.10:
import concurrent.{Future, Promise}
case class User(userId: Int)
case class UserDetails(userId: Int, schoolId: Option[Int])
case class School(schoolId: Int, name: String)
trait Error
class UserStore {
def getUserDetails(userId: Int): Future[Either[Error, UserDetails]] = Promise.successful(Right(UserDetails(1, Some(1)))).future
}
class SchoolStore {
def getSchool(schoolId: Int): Future[Option[School]] = Promise.successful(Option(School(1, "Big School"))).future
}
object Demo {
import concurrent.ExecutionContext.Implicits.global
val userStore = new UserStore
val schoolStore = new SchoolStore
val user = User(1)
val schoolFuture: Future[Option[School]] = for {
ud <- userStore.getUserDetails(user.userId)
sid = ud.right.toOption.flatMap(_.schoolId)
s <- sid.map(schoolStore.getSchool(_))
} yield s
}
(Edited to give a correct answer!)
The key here is that Future
and Option
don't compose inside for
because there aren't the correct flatMap
signatures. As a reminder, for desugars like so:
for ( x0 <- c0; w1 = d1; x1 <- c1 if p1; ... ; xN <- cN) yield f
c0.flatMap{ x0 =>
val w1 = d1
c1.filter(x1 => p1).flatMap{ x1 =>
... cN.map(xN => f) ...
}
}
(where any if
statement throws a filter
into the chain--I've given just one example--and the equals statements just set variables before the next part of the chain). Since you can only flatMap
other Future
s, every statement c0
, c1
, ... except the last had better produce a Future
.
Now, getUserDetails
and getSchool
both produce Futures
, but sid
is an Option
, so we can't put it on the right-hand side of a <-
. Unfortunately, there's no clean out-of-the-box way to do this. If o
is an option, we can
o.map(Future.successful).getOrElse(Future.failed(new Exception))
to turn an Option
into an already-completed Future
. So
for {
ud <- userStore.getUserDetails(user.userId) // RHS is a Future[Either[...]]
sid = ud.right.toOption.flatMap(_.schoolId) // RHS is an Option[Int]
fid <- sid.map(Future.successful).getOrElse(Future.failed(new Exception)) // RHS is Future[Int]
s <- schoolStore.getSchool(fid)
} yield s
will do the trick. Is that better than what you've got? Doubtful. But if you
implicit class OptionIsFuture[A](val option: Option[A]) extends AnyVal {
def future = option.map(Future.successful).getOrElse(Future.failed(new Exception))
}
then suddenly the for-comprehension looks reasonable again:
for {
ud <- userStore.getUserDetails(user.userId)
sid <- ud.right.toOption.flatMap(_.schoolId).future
s <- schoolStore.getSchool(sid)
} yield s
Is this the best way to write this code? Probably not; it relies upon converting a None
into an exception simply because you don't know what else to do at that point. This is hard to work around because of the design decisions of Future
; I'd suggest that your original code (which invokes a filter) is at least as good of a way to do it.