Using if-guard in for-comprehension to check for existence

scala for comprehension if else
scala if/else alternative
scala nested for comprehension
scala for loop
scala for comprehension assignment
scala for loop with condition
for loop in scala spark
scala match ( case if-else)

My am making 3 database queries, each return a Future. I am trying to use for comprehension to resolve the Futures but it seems I am not using if correctly in for

Each query depends on result of previous one. I look for a token, if found, I look for user and it found, I update the user. Each database query returns a Future[Option]] and I thought I could considitionally perform the next query depending on whether the previous one returns Some or None. I am using isDefined for this. But when I ran the code for an invalid token, I got error [NoSuchElementException: None.get] for code userOption:Option[User]<-userRepo.findUser(tokenOption.get.loginInfo); if tokenOption.isDefined

def verifyUser(token:String) = Action.async {
  implicit request => {
    val result:Future[Result] = for{
      //generator 1 - get token from database
      tokenOption:Option[UserToken] <- userTokenRepo.find(UserTokenKey(UUID.fromString(token)))
      //generator2. found token, look for corresponding user to which the token belongs
      userOption:Option[User] <- userRepo.findUser(tokenOption.get.loginInfo); if tokenOption.isDefined
      //generator 3. found user and token. Update profile 
      modifiedUser:Option[User] <-  confirmSignupforUser(userOption.get); if userOption.isDefined
       } yield 
         { //check if we have user and token and modified user here. If any is missing, return error else success
           if(tokenOption.isDefined && userOption.isDefined && modifiedUser.isDefined)
              Redirect("http://localhost:9000/home"+";signup=success")//TODOM - pick from config
           else
             if(tokenOption.isEmpty)
             Redirect("http://localhost:9000/home"+";signup=error")//TODOM - pick from config
           else if(userOption.isEmpty)
             Redirect("http://localhost:9000/home"+";signup=error")//TODOM - pick from config
           else if(modifiedUser.isEmpty)
             Redirect("http://localhost:9000/home"+";signup=error")//TODOM - pick from config
           else //this shouldn't happen. Unexpected
             Redirect("http://localhost:9000/home"+";signup=error")//TODOM - pick from config
         }
       result
     }
   }

TL;DR

Consider using OptionT https://typelevel.org/cats/datatypes/optiont.html


Have a look at my toned down implementation:

from https://scastie.scala-lang.org/hsXXtRAFRrGpMO1Jl1Li7A

import scala.concurrent.Future
import scala.concurrent.ExecutionContext.Implicits.global
import scala.concurrent.Await.result
import scala.concurrent.duration._
import scala.language.postfixOps

type UserToken = String
type User = String

def fromToken(token: String): Future[Option[UserToken]] = Future.successful(None)
def findUser(userToken: UserToken): Future[Option[User]] = Future.successful(None)
def modify(user: User): Future[Option[User]] = Future.successful(None)

def verifyUser(token: String) = {
  val result = for {
    tokenOption: Option[UserToken] <- fromToken(token) //generator 1 - get token from database
    userOption: Option[User] <- findUser(tokenOption.get);
    if tokenOption.isDefined //generator2. found token, look for corresponding user to which the token belongs
    modifiedUser: Option[User] <- modify(userOption.get);
    if userOption.isDefined //generator 3. found user and token. Update profile
  } yield { //check if we have user and token and modified user here. If any is missing, return error else success
    if (tokenOption.isDefined && userOption.isDefined && modifiedUser.isDefined)
      println("happy")
    else
      println("sad")
  }
  result
}

result(verifyUser("hello"), 1 second)

I used the following compile time flags, the last one is important:

scalacOptions ++= Seq(
  "-deprecation",
  "-encoding", "UTF-8",
  "-feature",
  "-unchecked",
  "-Xprint:typer"
)

Let's focus on this line of the compile output:

(((tokenOption: Option[Playground.this.UserToken]) => Playground.this.findUser(tokenOption.get).
withFilter(((check$ifrefutable$2: Option[Playground.this.User]) => (check$ifrefutable$2: Option[Playground.this.User] @unchecked) match {
      case (userOption @ (_: Option[Playground.this.User])) => true
      case _ => false
...

You can see that the tokenOption.get is invoked before the withFilter. These gets are the source of the exception you get

The, almost complete output of the compile is:

[[syntax trees at end of                     typer]] // main.scala
....
    import scala.concurrent.Future;
    import scala.concurrent.ExecutionContext.Implicits.global;
    import scala.concurrent.Await.result;
    import scala.concurrent.duration._;
    import scala.language.postfixOps;
    type UserToken = String;
    type User = String;
    def fromToken(token: String): scala.concurrent.Future[Option[Playground.this.UserToken]] = scala.concurrent.Future.successful[None.type](scala.None);
    def findUser(userToken: Playground.this.UserToken): scala.concurrent.Future[Option[Playground.this.User]] = scala.concurrent.Future.successful[None.type](scala.None);
    def modify(user: Playground.this.User): scala.concurrent.Future[Option[Playground.this.User]] = scala.concurrent.Future.successful[None.type](scala.None);
    def verifyUser(token: String): scala.concurrent.Future[Unit] = {
      val result: scala.concurrent.Future[Unit] = Playground.this.fromToken(token).withFilter(((check$ifrefutable$1: Option[Playground.this.UserToken]) => (check$ifrefutable$1: Option[Playground.this.UserToken] @unchecked) match {
  case (tokenOption @ (_: Option[Playground.this.UserToken])) => true
  case _ => false
}))(scala.concurrent.ExecutionContext.Implicits.global).flatMap[Unit](((tokenOption: Option[Playground.this.UserToken]) => Playground.this.findUser(tokenOption.get).withFilter(((check$ifrefutable$2: Option[Playground.this.User]) => (check$ifrefutable$2: Option[Playground.this.User] @unchecked) match {
  case (userOption @ (_: Option[Playground.this.User])) => true
  case _ => false
}))(scala.concurrent.ExecutionContext.Implicits.global).withFilter(((userOption: Option[Playground.this.User]) => tokenOption.isDefined))(scala.concurrent.ExecutionContext.Implicits.global).flatMap[Unit](((userOption: Option[Playground.this.User]) => Playground.this.modify(userOption.get).withFilter(((check$ifrefutable$3: Option[Playground.this.User]) => (check$ifrefutable$3: Option[Playground.this.User] @unchecked) match {
  case (modifiedUser @ (_: Option[Playground.this.User])) => true
  case _ => false
}))(scala.concurrent.ExecutionContext.Implicits.global).withFilter(((modifiedUser: Option[Playground.this.User]) => userOption.isDefined))(scala.concurrent.ExecutionContext.Implicits.global).map[Unit](((modifiedUser: Option[Playground.this.User]) => if (tokenOption.isDefined.&&(userOption.isDefined).&&(modifiedUser.isDefined))
        scala.Predef.println("happy")
      else
        scala.Predef.println("sad")))(scala.concurrent.ExecutionContext.Implicits.global)))(scala.concurrent.ExecutionContext.Implicits.global)))(scala.concurrent.ExecutionContext.Implicits.global);
      result
    };
    scala.Predef.locally[Unit]({
      val $t: Unit = scala.concurrent.Await.result[Unit](Playground.this.verifyUser("hello"), scala.concurrent.duration.`package`.DurationInt(1).second);
      Playground.this.instrumentationMap$.update(com.olegych.scastie.api.Position.apply(1199, 1236), com.olegych.scastie.api.runtime.Runtime.render[Unit]($t)((ClassTag.Unit: scala.reflect.ClassTag[Unit])));
      $t
    })
  };
  object Main extends scala.AnyRef {
    def <init>(): Main.type = {
      Main.super.<init>();
      ()
    };
    private[this] val playground: Playground = new Playground();
    <stable> <accessor> def playground: Playground = Main.this.playground;
    def main(args: Array[String]): Unit = scala.Predef.println(com.olegych.scastie.api.runtime.Runtime.write(Main.this.playground.instrumentations$))
  }
}

How to use a Scala `for` loop with embedded `if` statements (guards , This is Recipe 3.3, “How to use a for loop with embedded if statements (guards).” Problem. You want to add one or more conditional clauses to a  This is Recipe 3.13, “How to add if expressions (guards) to match/case expressions.” Problem. You want to add qualifying logic to a case statement in a Scala match expression, such as allowing a range of numbers, or matching a pattern, but only if that pattern matches some additional criteria. Solution. Add an if guard to your case statement. Use it to match a range of numbers:

I am not sure why you are surprised you are getting and error for None.get with invalid token: if token is invalid, tokenOption is None, so, the next statement tokenOption.get will fail with exactly this error.

You want the "guard" executed before the statement you want to short circuit, not after it:

for {
   foo <- bar if foo.isDefined
   baz <- foo.get
 } yield baz

But this would fail in the end anyway, because there would be nothing to yield (this trick works with Options or Lists etc., but Future.withFilter will end up failing if predicate is not satisfied, there is no other alternative).

The general rule to avoid this kind of errors is never use .get on an Option (or on a Try). Also, never use .head on a List, .apply on a Map, etc.

Here is one (almost) idiomatic way to write what you want:

case object Error extends RuntimeException("")
userTokenRepo
  .find(UserTokenKey(UUID.fromString(token)))
  .map { _.getOrElse(throw Error)
  .flatMap { userRepo.find(_.loginInfo) }
  .map { _.getOrElse(throw Error) }
  .flatMap(confirmSignupForUser)
  .map { _.getOrElse(throw Error) }
  .map { _ => "success") }
  .recover { case Error => "error" }
  .map { result => Redirect(s"http://localhost:9000/home;signup=$result" }

Note, I said this was "almost" idiomatic, because throwing exceptions in scala is frowned upon. A purist would object to it, and suggest using something like a Try . or a biased Either instead, or to make use of a third party library, like cats or scalaz, that provide additional tools for working with Futures of Option (namely, OptionT).

But I would not recommend getting into that right now. You should get comfortable enough with basic "vanilla" scala before starting with that advanced stuff to avoid ending up with something completely incomprehensible.

You could also write this differently, in a completely idiomatic way (without using exceptions), with something like this:

  userTokenRepo.find(UserTokenKey(UUID.fromString(token)))
    .flatMap { 
        case Some(token) => userRepo.find(token.loginInfo)
        case None => Future.successful(None) 
     }.flatMap { 
        case Some(user) => confirmSignupForUser(user)
        case None => Future.successful(None)
     }.map {
        case Some(_) => "success"
        case None => "error"
     }.map { result => 
        Redirect(s"http://localhost:9000/home;signup=$result"     
     }

This is more "pure", but a little more repetitive, so my personal preference is the first variant.

Finally, you could do away with my Error thingy, and just handle the NoSuchElement exception directly. This is going to be the shortest, but kinda icky even to my taste (what if some downstream code throws this exception because of a bug?):

 userTokenRepo
   .find(UserTokenKey(UUID.fromString(token)))
   .flatMap { userRepo.find(_.get.loginInfo) }
   .flatMap(confirmSignupForUser(_.get))
   .map { _.get }
   .map { _ => "success") }
   .recover { case _: NoSuchElementException => "error" }
   .map { result => 
     Redirect(s"http://localhost:9000/home;signup=$result" 
   }

I really don't recommend the last version though, despite it being the shortest, and arguably, the most readable one (you can even rewrite it with a for-comprehension to look even nicer). Using Option.get is commonly considered "code smell", and is almost never a good thing to do.

Scala: How to add 'if' expressions (guards) to match/case , Add an if guard to your case statement. Use it to match a range of numbers: i match { case a if 0 to 9 contains a => println("0-9 range: " + a) case  Set comprehensions allow sets to be constructed using the same principles as list comprehensions, the only difference is that resulting sequence is a set. Say we have a list of names. The list can contain names which only differ in the case used to represent them, duplicates and names consisting of only one character.

Motivated by How to best handle Future.filter predicate is not satisfied type errors

I rewrote like the following. While the code works, I am curious to know if I am doing it the right way (functional!). Does it look fine?

   def verifyUser(token:String) = Action.async {
     implicit request => {
       println("verifyUser action called with token: " + token) //TODOM - add proper handling and response

       val result:Future[Result] = for{tokenOption:Option[UserToken] <- userTokenRepo.find(UserTokenKey(UUID.fromString(token)))  //generator 1 - get token from database
                                    userOption:Option[User] <- if (tokenOption.isDefined) userRepo.findUser(tokenOption.get.loginInfo) else Future.successful(None) //generator2. found token, look for corresponding user to which the token belongs
                                    modifiedUser:Option[User] <- if (userOption.isDefined) confirmSignupforUser(userOption.get) else Future.successful(None) //generator 3. found user and token. Update profile
                                    deletedToken:Option[UserTokenKey] <- if(modifiedUser.isDefined) userTokenRepo.remove(UserTokenKey(UUID.fromString(token))) else Future.successful(None)
       }
         yield { //check if we have user and token and modified user here. If any is missing, return error else success
           println("db query results tokenOption: "+tokenOption+", userOption: "+userOption+" : modifiedUserOption: "+modifiedUser+", deletedToken: "+deletedToken)
           if(tokenOption.isDefined && userOption.isDefined && modifiedUser.isDefined && deletedToken.isDefined)
              Redirect("http://localhost:9000/home"+";signup=success")//TODOM - pick from config
           else
             if(tokenOption.isEmpty)
             Redirect("http://localhost:9000/home"+";signup=error")//TODOM - pick from config
           else if(userOption.isEmpty)
             Redirect("http://localhost:9000/home"+";signup=error")//TODOM - pick from config
           else if(modifiedUser.isEmpty)
             Redirect("http://localhost:9000/home"+";signup=error")//TODOM - pick from config
           else //this shouldn't happen. Unexpected
             Redirect("http://localhost:9000/home"+";signup=error")//TODOM - pick from config
         }
       result
     }
   }

Games in Libraries: Essays on Using Play to Connect and Instruct, Essays on Using Play to Connect and Instruct Breanne A. Kirsch In this case, library staff could be guards that deduct points if they catch students running. They suggested Gotham (if such a map exists), Narnia, Middle Earth, or Eragon. the comprehension check on this important aspect of using scholarly databases. One argument against using guard is that it encourages large and less testable functions by combining tests for multiple values all in the same place. If used naïvely this could be true but with the proper use, guard allows us to smartly separate concerns, letting the view controller deal with managing the view elements while the validation for these elements can sit in a fully tested validation class or extension.

Programming Languages and Systems: 12th Asian Symposium, APLAS , Our heuristics also accounts for indexing guards and early scheduled guards: a constraints and comprehensions ̄Ha and ̄Hc and guards ̄g, we compute a substitution φ such that φA = A if it exists and ⊥ otherwise; lookupCands(Ls,A If there is at least one such candidate (φ, A#m), the search proceeds with it as  Each for comprehension begins with a generator. for comprehensions will be multiple generators. Definitions - For comprehension definitions have below syntax: pattern = expression. For example n = b.name the variable n is bound to the value b.name. That statement has a similar result as writing this code outside of a for comprehension. val n

Delver Magic Book X: Search and Discover, Burbon's Captain of the Guard maintained an uneasy watch over a meeting he did not fully The very existence of a demon lord shook his convictions. It was as if the monster was holding up a mirror that only the captain could see. Even after the delver disappeared with the sorceress, he reassured his guards that the​  of the whole system of things, which we manifestly have not, and mere faith grounded on probability. Dew, Review of the Debate in the Virginia Legislature, 1831-32 (Richmond, 1832), important for a. of the slavery issue; J. It was eminently a doctrine of. and of toleration. This general case has been discussed at length because a careful study

The Epworth Herald, ought to be able to double his collection if he secures |. emblem of purity. One at the head.—As if guard-| oo VERY successful organization must exist for a purpose. With that amount of money annually added to what we now have, within ten And yet we do not cease to look to the church at home, in the eager hope that  Chain optional values together using for-comprehension. Introduce conditions within a for-comprehension statement; Code using the most common operations defined on Option; You can use the functions map, flatten, and flatMap to manipulate optional values.

Comments
  • This is going to require a lot of rewriting. I think you should spend a little time getting used to using Option types. You shouldn't need to do any of these .isEmpty or .isDefined calls. You will want to just pass the option around in such a way that at then end of your computation, you'll either have a Some(result) or a None. Then, at that point, handle the None case. A large if/else block in your yield is also unusual. Instead, return the value there, and then as a subsequent step use Pattern Matching to return the correct Redirect
  • But changing function signatures is not in my control. If know I could write the above code using ‘map’ and ‘flatMap’ but that code becomes too nested and unreadable. I should be able to write the logic using ‘for’ as well
  • I'm not suggesting changing the signature. I'm suggesting you get rid of all the "if x.isDefined" and "if x.isEmpty" entirely. If one of your Option's is None, it will simply pass that None along. Then at the end, handle the None case. Don't check for None at every step.
  • sorry but I am struggling to understand how I can remove a check if I need it. userRepo.findUser(tokenOption.get.loginInfo) doesn't accept an Option so I can't pass None to it and I have to get the loginInfo from tokenOption and pass it. But I don't know whether the token is valid or not so I need to check it before passing it to findUser.
  • I understand what you are saying but am unable to translate it to code. Eg, I see now that the if isn't working because because it translates to userRepo.findUser(tokenOption.get.loginInfo).withFilter and not if (tokenOption.isDefined) {userRepo.findUser(tokenOption.get.loginInfo)}
  • Thanks but considering that I am new to Scala, I'll prefer to first learn it property. cats might be too much for me at the moment.
  • I have been new to scala for over 3 years. All I can recommend is keep on challenging yourself - there is always something new to learn - the scala ocean is vast, don't let that deter you.
  • Thanks. I am able to write the code using map and flatMap but I wanted to give for a try as I thought it will make the code more readable. Please see the way I have done it now (my answer below). Does it look OK?
  • @ManuChadha well, as I mentioned earlier, calling .isDefined and especially .get on Option is "code smell", so ... not really. This just isn't a good use case of a for-comprehension (except if you combine it with monad transformers).
  • Do you think that I should use ‘for’ if I need to resolve multiple futures which don’t depend on one another? If they do then map/flatmap are better option?
  • If they don't depend on one another, you can just traverse or sequence them. That way they'd be run in parallel. for-comprehension is the really same thing as a chain of flatMaps, it just sometimes one form reads than the other. In particular, for comprehension is helpful, when some of the futures in the chain depend on several of preceding results. It may get cumbersome to express that with .flatMap. If you can write it so that each next future only needs the previous result, my personal preference is .flatMap.
  • Don't throw Error! Error is reserved for unrecoverable errors, like OOM. Throw Exception.
  • right (functionl!) and idiomatic are contentious words. My personal preference for this kind of situation, as I had mentioned in my answer, are monad transformers. But, others would disagree
  • You can replace if(foo.isDefined) stuff(foo.get) else Future.successful(None) with foo.fold(Future.successful(None))(stuff(_)). Doing .get on an option smells really badly. Also in the yield clause, four error branches are exactly the same. If you can just do Redirect(url + deletedToken.fold("success")(_ => "error")) instead of the whole if thingy