Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

How do I make these line of codes more scala-ish (shorter?). I still get the Java feeling in it (which I want to stay away from).

import scala.collection.mutable
val outstandingUserIds: mutable.LinkedHashSet[String] = mutable.LinkedHashSet[String]()    

val tweetJson = JacksMapper.readValue[Map[String, AnyRef]](body)     
val userObj = tweetJson.get("user")    
tweetJson.get("user").foreach(userObj => {
  userObj.asInstanceOf[Map[String, AnyRef]].get("id_str").foreach(idStrObj => {
    if (outstandingUserIds.exists(outstandingIdStr => outstandingIdStr.equals(idStrObj))) {
      outstandingUserIds.remove(idStrObj.asInstanceOf[String])
    }
  })
})
share|improve this question
    
Welcome to Code Review! To make life easier for reviewers, please add sufficient context to your question. The more you tell us about what your code does and what the purpose of doing that is, the easier it will be for reviewers to help you. See also this meta question –  Simon André Forsberg Jun 11 at 20:44

2 Answers 2

up vote 8 down vote accepted

If you really want to inflict this JSON library on yourself, your code could be simplified to:

import scala.collection.mutable

val outstandingUserIds = mutable.LinkedHashSet[String]()    

val tweetJson = JacksMapper.readValue[Map[String, AnyRef]](body)     
for {
  userObj <- tweetJson.get("user")
  idStrObj <- userObj.asInstanceOf[Map[String, AnyRef]].get("id_str")
} outstandingUserIds -= idStrObj.asInstanceOf[String]

But I would recommend you use a better JSON library. For example, using the library that comes with the Play! framework:

(body \ "user" \ "id_str").asOpt[String].foreach {id =>
  outstandingUserIds -= id
}
share|improve this answer
    
I'm not developing web-app. I'm developing a little daemon process. Can I use Play for that? –  user3443096 Jun 11 at 21:30
    
Yes. Even better, you can just include the Play JSON library. See this answer. –  wingedsubmariner Jun 11 at 23:46
    
Thanks for the suggestion, I just tried it, turned out Play framework uses Scala 2.10, while I'm using Scala 2.11 (I'm getting this error in Eclipse: akka-actor_2.10-2.3.3.jar is cross-compiled with an incompatible version of Scala (SpecificScalaVersion(2,10,0,Final))) Hmm... second thought, maybe I'll just switch back to Scala 2.10.... –  user3443096 Jun 12 at 0:49

You usually want to avoid mutable collections in Scala, unless you have a good reason.

val outstandingUserIds: Set[String] = ...  // immutable

val userId: Option[String] = tweetJson.get("user")
           .flatMap(userObj => userObj.asInstanceOf[Map[String, AnyRef]].get("id_str"))
           .map(_.asInstanceOf[String])
userId.foreach(id => outstandingUserIds = outstandingUserIds - id)

I wrote the type Option[AnyRef] just for clarity here; you probably would not write it in production code.

You can subtract an element from a Set even if it is not present.

The constant casting is quite annoying. Maybe someone who has experience with such a json library will have better suggestions.

share|improve this answer
    
Hi, Thanks, it works. My understanding: (1) userObj.asInstanceOf[Map[String, AnyRef]].get("id_str") returns an instance of Option[AnyRef], which is a kind of collection. So by using flatMap, we take that AnyRef out of the collection, and put it in the resulting list (that's why we use flatMap). (2) The outcome of that flatMap, we get a a list of AnyRef ( List[AnyRef] ...) (3) The outcome of map on that list of AnyRef is a list of String (we type-cast each element in the list). Hmm..., can we say that List[String] is a subtype of Option[String] ? That's what it seems from this snippet. –  user3443096 Jun 11 at 21:28
    
Duh, how do I format I my comment? –  user3443096 Jun 11 at 21:31
    
You can't format comments, except for the back ticks to highlight code. –  toto2 Jun 11 at 21:43
    
Option is not really a collection, but a monad. Option.flatMap(f: A => Option[B]) returns an Option that is None if the original Option is None, or if the transformation with f returns None. So you can basically chain transformations (you can chain more than two), and if one transformation goes wrong somewhere, you get None in the end. –  toto2 Jun 11 at 21:52
    
By the way, I realized that since I have a flatMap followed by a map, I should have used a for-comprehension like wingedsubmariner. A for-comprehension in Scala is just syntactic sugar for sequences of maps and flatMaps (and filters). –  toto2 Jun 11 at 21:54

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.