0

this thread gave me an idea how to structure my code: Scala-way to handle conditions in for-comprehensions?

The part in question:

// First create the JSON
val resultFuture: Future[Either[Failure, JsResult]] = for {
  userRes <- userDao.findUser(userId)
  user    <- userRes.withFailure(UserNotFound).right
  authRes <- userDao.authenticate(user)
  auth    <- authRes.withFailure(NotAuthenticated).right
  goodRes <- goodDao.findGood(goodId)
  good    <- goodRes.withFailure(GoodNotFound).right
  checkedGood <- checkGood(user, good).right
} yield renderJson(Map("success" -> true)))

This are the lines I do not understand:

user    <- userRes.withFailure(UserNotFound).right
authRes <- userDao.authenticate(user)

The userRes.withFailure(UserNotFound).right is mapped to userDao.authenticate(user). This will create a new Either with a Future on its right, correct?

How can

val resultFuture: Future[Either[Failure, JsResult]]

be of its type. I think instead of a JsResult there should be another future. Can anyone explain this to me?

EDIT: Since cmbaxter and Arne Claassen confirmed this, the new question is: How should I write this code, so it does not look ugly, but clean and structured?

Community
  • 1
  • 1
Tim Joseph
  • 847
  • 2
  • 14
  • 28
  • I agree that there is something wrong with that answer. As has been posted on SO many times, you can not mix types in a for-comprehension. Whatever type you start with is the type that you need to propagate through the rest of the steps (`flatMap`s). In this case, because `findUser` returns a `Future[Option[User]]` then `Future` becomes the type that you must continue to use in the rest of the steps. You can't just switch to an `Either` as this person does in the second step of the for-comprehension – cmbaxter Apr 29 '15 at 17:02
  • Little confused myself. Generally a for comprehension works on one type of Monad at a time and here we're mixing functions that produce `Future`s and `Either`s. I've not been able to get the original example to parse – Arne Claassen Apr 29 '15 at 17:17

1 Answers1

4

I believe the answer you received needlessly mixed Either's into the mix when Future's are already perfectly capable of communicating failure. The main thing you were missing was a way to get from an Option to the option's value without explicitly throwing exceptions.

I would suggest that you change the Failures object to the following:

object Failures {

  sealed trait Failure extends Exception

  // Four types of possible failures here
  case object UserNotFound extends Failure

  case object NotAuthenticated extends Failure

  case object GoodNotFound extends Failure

  case object NoOwnership extends Failure

  // Put other errors here...

  // Converts options into Futures
  implicit class opt2future[A](opt: Option[A]) {
    def withFailure(f: Failure) = opt match {
      case None => Future.failed(f)
      case Some(x) => Future.successful(x)
    }
  }
}

Now you can map a Future[Option[A]] to a Future[A] and specify the failure condition, resulting in a for comprehension like this:

def checkGood(user: User, good: Good) =
  if (checkOwnership(user, good))
    Future.successful(good)
  else
    Future.failed(NoOwnership)

val resultFuture: Future[JsResult] = for {
  userOpt <- userDao.findUser(userId)
  user <- userOpt.withFailure(UserNotFound)
  authOpt <- userDao.authenticate(user)
  auth <- authOpt.withFailure(NotAuthenticated)
  goodOpt <- goodRes.withFailure(GoodNotFound)
  checkedGood <- checkGood(user, good)
} yield renderJson(Map("success" -> true))))

Now that you have a Future[JsResult] you can map the failed scenarios to your desired output and the success scenario is just the JsResult. Hopefully you are using this in an asynchronous framework which expects you to feed it a future and has its own failed future to error response mapping (such as Play!).

Arne Claassen
  • 14,088
  • 5
  • 67
  • 106
  • Is there a way to do this, without having to extend "Exception"? – Tim Joseph Apr 29 '15 at 18:27
  • 1
    Not any more cleanly. The failure condition of Future takes a Throwable. And if you don't fail the future you can't short-circuit out of the future for comprehension. Any particular reason to eschew Exception other than it makes you think of ugly Java try/catch flows? In addition, exceptions are the Play way of mapping request failure to response, so it is the idiomatic way to do it. – Arne Claassen Apr 29 '15 at 20:23
  • Okay, thank you very much. Its just that using Exceptions for expected behaviour does not feel right at all :D – Tim Joseph Apr 30 '15 at 15:15
  • I guess at least in this example, it isn't expected behavior, since the for comprehension presumes that each `DAO` step produces a value to feed into the next, so getting a `None` at least for this workflow is unexpected behavior – Arne Claassen Apr 30 '15 at 16:08
  • user <- userOpt.withFailure(UserNotFound): to me is expected behaviour. Unexpected behaviour would be a connection loss to my database for example... – Tim Joseph Apr 30 '15 at 19:40
  • In that case, the above for comprehension is not the workflow you are looking for, as it implies success if all the actions can occur that depend on each other and what you really seem to want is a branching workflow. Let me think about it and if i come up with an elegant way, i'll post it on your original question – Arne Claassen Apr 30 '15 at 20:28