2

i'm writing a Play 2.3.2 application in Scala. I use the reactivemongo driver to access to a mongodb database. I've write that query the db to obtain the most used tag in the database. This method is an Action.async and is implemented as the following:

def max = Action.async { request =>

          var max: Int = 0
          var tagFound: Tag = null
          //obtain all the tags in the db.
          val futureTags: Future[List[Tag]] = Tags.all.toList
          futureTags map{ (tags: List[Tag]) => 
            tags map { (tag: Tag) => 
              //create the tag String 
              val tagName = tag.category  + ":" + tag.attr 
              //search in the db the documents where tags.tag == tag.
              val futureRequests : Future[List[recommendationsystem.models.Request]]= Requests.find(Json.obj("tags.tag" -> tagName)).toList
              futureRequests map { (requests: List[recommendationsystem.models.Request]) =>
                //get the numbers of documents matching the tag
                val number: Int= requests.size
                if(number > max) {
                  max = number
                  tagFound = tag
                }
                println(max)
              }
            }   

         val jsonObject = if(max > 0) Json.obj("tag" -> tagFound, "occurencies" -> max) else Json.obj("tag" -> "NoOne", "occurencies" -> 0)
         Ok(jsonObject)
         }


      }

But the behavior of this method is not deterministic, what's wrong?? I can't understand why the

val jsonObject = if(max > 0) Json.obj("tag" -> tagFound, "occurencies" -> max) else Json.obj("tag" -> "NoOne", "occurencies" -> 0)
             Ok(jsonObject)
             }

is execute asynchronous and don't wait that the tags map statement finished.

alberto adami
  • 729
  • 1
  • 6
  • 25

2 Answers2

1

I have found a few problems with your code:

  1. don't user vars, you don't know when they will be updated, hence nondeterminism. In fact don't use vars at all
  2. you use map as foreach, i.e you don't return any value from map, use foreach explicitly if you don't return any value from future
  3. to flatten Future[Future[T]] use flatMap,
  4. to transform List[Future[T]] to Future[List[T]] use Future.sequence

Here's a rewritten piece of code, I haven't compiled it, but you can get an idea of how it should work:

def max = Action.async { request =>
    Tags.all.toList.flatMap { case tags =>
      val xs = tags map { case tag =>
        val tagName = tag.category  + ":" + tag.attr 
        Requests.find(Json.obj("tags.tag" -> tagName)).toList.map (requests => (tag, requests.size) )
      }
      val f = Future.sequence(xs)
      f.map { case ys =>
         val res = ys.foldLeft(Option.empty[(Tag, Int)]) { 
            case (Some((maxTag, maxOcc)), (tag, occ)) if occ > maxOcc => Some(tag, occ)
            case (s@Some(_), _) => s
            case (None, (tag, occ)) => Some(tag, occ)
         }
         val jsonObject = res.map { case (tag, maxOcc) =>
           Json.obj("tag" -> tagFound, "occurencies" -> maxOcc)
         } getOrElse {
             Json.obj("tag" -> "NoOne", "occurencies" -> 0)
         }
         Ok(jsonObject)
      }

    }
}
vitalii
  • 3,335
  • 14
  • 18
  • Thanks for your reply, but i can't understand the foldLeft part of the code. Can you explain what are you doing?? Otherwise the code don't compile, in my @edit you can see the compiler errors. – alberto adami Oct 10 '14 at 10:13
  • Compiler errors meant as an exercise to the reader. I will fix the easy ones – vitalii Oct 10 '14 at 13:50
  • foldleft here just computes maximum "occurrencies" number and corresponding tag. You can use mutable variables instead, as in the question – vitalii Oct 10 '14 at 13:54
0

Future.map returns another Future - it doesn't make the future run immediately. After you call f.map(...), f may not yet have completed.

If you want to block until a Future completes, use Future.get (So you could do f.map(...).get, though in that case you might as well do f.get and do a computation on that value - of course this will not be nonblocking). If you want to chain together several async computations you can use flatMap and sequence, and should try to avoid using vars in this case, as per @vitalii's answer.

The -Ywarn-value-discard compiler flag can warn you when you're discarding a value, which usually indicates a problem with the program.

lmm
  • 17,386
  • 3
  • 26
  • 37