How to fix a warning on asInstanceOf usage

I'm using scapegoat for Scala static code analysis and I'm getting a warning on a piece of code. Here is the full warning

Warning  Use of asInstanceOf  com.sksamuel.scapegoat.inspections.unsafe.AsInstanceOf
asInstanceOf used near cursor.next().asInstanceOf[com.mongodb.casbah.Imports.BasicDBObject]. Consider using pattern matching.

The line the warning points to is

obj = cursor.next().asInstanceOf[BasicDBObject]

Which belongs to this piece of code

val q = QueryBuilder.start(fieldName.toString()).is(value)
val cursor = collectionMongo(ARTGROUP_COLLECTION_NAME).find(q.get)
var obj = new BasicDBObject
try {
  while (cursor.hasNext) {
    obj = cursor.next().asInstanceOf[BasicDBObject]
    log.debug(" obj.tostring" + obj.toString())
    retunedList += parseArtGroup(obj)
   }
} catch {
}

How can I use pattern matching in the code above?

Solution

Scala stresses type safety a lot, more so than most widespread languages, which is why casting is often seen as a code smell. For the very same reason, the language designer decided to make casting arguably awkward with similarly named isInstanceOf[T] and asInstanceOf[T] to query a type at runtime and casting it.

To overcome this while still being able to interact with not-so-type-safe libraries, pattern matching is often suggested.

Here is your snippet of code with pattern matching instead of casting:

val q = QueryBuilder.start(fieldName.toString()).is(value)
val cursor = collectionMongo(ARTGROUP_COLLECTION_NAME).find(q.get)
var obj = new BasicDBObject
try {
  while (cursor.hasNext) {
    cursor.next() match {                // HERE
      case basicDbObj: BasicDBObject =>  // HERE
        obj = basicDbObj                 // HERE
    }    
    log.debug(" obj.tostring" + obj.toString())
    retunedList += parseArtGroup(obj)
  }
} catch {
}

Pattern matching is a Scala feature that allows you to apply something akin to switch/case constructs in other languages but with more expressive semantics.

Pattern matching also allows you, among other things, to deconstruct your input in meaningful ways, for example:

List(1, 2, 3, 4) match {
  case head :: tail =>
    println(s"The first element is $head and then comes $tail")
}

It's worth mentioning that if you don't cover all possible cases you may get a different warning as you may throw a MatchError if no matching clause is satisfied.

In case you cannot fully cover all possible cases, you may want to consider the _ token that symbolizes the wildcard catch all pattern as in the following example:

cursor.next() match {
  case basicDbObj: BasicDBObject =>
    obj = basicDbObj
  case _ => // default case
    ??? // probably some error handling
}

You can read more on pattern matching in Scala in the official documentation. It's a very well written document and you'll learn a lot about this very powerful feature of Scala.

One nice thing I'd like to add at this point is that Scala's try/catch construct uses an analogous syntax.

try {
  throw new RuntimeException("kaboom :)")
} catch {
  case e: RuntimeException =>
    println(e.getMessage) // prints "kaboom :)"
}

If you are unsure about what you want to catch, Scala ships with a very useful function to destructure non fatal exceptions:

import scala.util.control.NonFatal

try {
  throw new RuntimeException("kaboom again!")
} catch {
  case NonFatal(e) =>
    println(e.getMessage) // prints "kaboom again!"
}

Quoting the official documentation:

Extractor (note: more on extractors here) of non-fatal Throwables. Will not match fatal errors like VirtualMachineError (for example, OutOfMemoryError and StackOverflowError, subclasses of VirtualMachineError), ThreadDeath, LinkageError, InterruptedException, ControlThrowable.

You may want to use something similar to this in your code.

On a different note, it looks like in your code you are parsing the objects in an iterator and adding them to a list. It goes beyond your question, but I'd like to offer a small suggestion.

You may want to look into using something like the following to do that:

import scala.util.Try

Try(cursor.collect { case o: BasicDBObject => parseArtGroup(o) }).foreach(returnedList ++= _)

It may actually be the case that you don't actually need to append your results to returnedList, but I'll let you be the judge of that as I don't know your code. If you think this approach makes sense, you can read more on Try here and learn about the collect method here.