scalascalastyle

Scala case-match optimization for performance and readability


I wrote the below for pattern matching in scala :

scala> val rowMap = Map("abc" -> null)
rowMap: scala.collection.immutable.Map[String,Null] = Map(abc -> null)

scala> val postgresKey = "abc"
postgresKey: String = abc

scala> val h2Key = "xyz"
h2Key: String = xyz

Suggested Option:

scala> (rowMap.get(postgresKey).map(_.asInstanceOf[String]) , rowMap.get(h2Key).map(_.asInstanceOf[String])) match
     |       {
     |         case (Some(value), _) if (value != null && value.trim.nonEmpty) => value
     |         case (None, Some(value)) if (value != null && value.trim.nonEmpty) => value
     |         case (_ , _) => "0.0"
     |       }
res9: String = 0.0

My Code :

scala> (rowMap.get(postgresKey).map(_.asInstanceOf[String]) , rowMap.get(h2Key).map(_.asInstanceOf[String])) match
     |       {
     |         case (Some(value), _) if (value != null ) => if (value.trim.nonEmpty) value else "0.0"
     |         case (None, Some(value)) if (value != null ) => if (value.trim.nonEmpty) value else "0.0"
     |         case (_ , _) => "0.0"
     |       }  
res10: String = 0.0

Does the suggested option have any performance benefits than my code. I know that && will stop comparing the value.trim.nonEmpty the moment it sees a null. So, we save on 1 comparison.

I do understand that it is cleaner to read as well. Anything else that is better in the suggested approach ?

EDIT 1: I have been told to avoid using null. However it is coming as an input to me from anorm's defaultParser. Since the scenario is difficult to replicate, I gave the example above. In my case, unit tests are in H2 and actual database is postgres.

Below is the snippet from the code:

            val anormQuery = SQL(query)
            // map the anorm Row as per the input params, differentiate into aggregated and group cols
            val tmp = anormQuery.as(anormQuery.defaultParser.*)

            logger.info(" Printing the result set object " + tmp.toString())

            val finalSqlResultset = tmp.map(row ⇒ {

              // Anorm row.asMap has this behaviour that it adds either a leading dot(.) or <tablename>. in front of the map keys (the columns/alias in sql) based on whether it is H2 or Postgres

              val rowMap = for ((k, v) ← row.asMap) yield (k, v)
              logger.info("Print the resultSet as map : " + rowMap.toString())


              val aggregates = expressionsToAggregate.map(input ⇒ {
                val (anormKey, postgresKey) = doesColumnHaveAlias.get(input.alias.getOrElse("UnknownAlias")).getOrElse(("Unknown", "Unknown"))
                //
                val aggResult = AggregatedValue(
                  input.alias.get,
                  (rowMap.get(postgresKey).map(_.asInstanceOf[String]), rowMap.get(anormKey).map(_.asInstanceOf[String])) match {
                    case (Some(value), _) if (value != null && value.trim.nonEmpty) ⇒ value
                    case (None, Some(value)) if (value != null && value.trim.nonEmpty) ⇒ value
                    case (_, _) ⇒ "0.0"
                  })
                logger.info("## TRACE 1 ##" + aggResult)
                aggResult
              })
})

Solution

  • First of all, don't use nulls. Ever. Just do val rowMap = Map("abc" -> "").

    Or better yet, just not put junk into map at all (you are filtering it out anyway!).

    Second, since you mentioned readability, it looks like you are looking for something like this:

     rowMap.get(postgresKey).filterNot(_.trim.isEmpty)
      .orElse(rowMap.get(h2Key).filerNot(_.trim.isEmpty))
      .getOrElse("0.0")
    

    Finally, to answer your question the two versions you showed are not equivalent. The first one does what I did above. The second one is this:

     rowMap.get(postgresKey).orElse(rowMap.get(h2Key))
       .filterNot(_.trim.isEmpty)
       .getOrElse("0.0")
    

    In the former case, setting postgresKey to "" is equivalent to not setting it at all, you'll always get the value for h2Key in that case. In the latter case, if postgresKey is set to "" you will get "0.0", regardless of what h2Key is set.

    So, the behaviors are different, it depends on which one you actually need.

    If the first type of behavior is what you need, it can also be written like this:

    Some(rowMap.filterValues(_.trim.nonEmpty)).flatMap { case m => 
       m.get(postrgesKey) orElse m.get(h2Key)
    }.getOrElse("0.0")
    

    Or, if you listen to my advise and decide to not put junk into the map in the first place, you can loose the filter too:

    roMap.get(postrgesKey) orElse rowMap.get(h2Key) getOrElse "0.0"