Need help to refactor this scala method in functional programming style

Question

I have this scala method that build a Map from some parameters:

def foo(name: Option[String], age: Option[Int], hasChilds: Option[Boolean], 
    childs: Option[List[Map[String, Any]]]): Map[String,Any] = {

    var m = Map[String, Any]()

    if (!name.isEmpty) m += ("name" -> name.get)
    if (!age.isEmpty) m += ("age" -> age.get)
    if (!hasChilds.isEmpty) m += ("hasChilds" -> hasChilds.get)
    if (!childs.isEmpty) m += ("childs" -> childs.get)

    m
}

I wonder if there is a way to refactor the code in more functional style?

Is it possible to eleminate the using of var in this case?

As a side note, hasChilds seems redundant if you're using an Option for childs. — Karl Bielefeldt
– Karl Bielefeldt, Commented Jan 16, 2015 at 18:05

elm · Accepted Answer · 2015-01-16 18:01:57Z

6

One approach includes the flattening of an immutable Map, like this,

def foo(name: Option[String], 
        age: Option[Int], 
        hasChilds: Option[Boolean], 
        childs: Option[List[Map[String, Any]]]): Map[String,Any] = {

  Map( ("name" -> name), 
       ("age" -> age),
       ("hasChilds" -> hasChilds),  
       ("childs" -> childs)).collect { case(a,Some(b)) => (a,b) }
}

edited Jan 16, 2015 at 18:01

answered Jan 16, 2015 at 17:49

elm

20.5k15 gold badges72 silver badges116 bronze badges

Sign up to request clarification or add additional context in comments.

4 Comments

null Over a year ago

Why the partial function case (a,Some(b)) => (a,b) inside the collect doesn't trigger MatchError if there is None value in one of parameters?

dk14 Over a year ago

that's how collect method works it takes only elements on which the function is defined. See isDefinedAt

elm Over a year ago

@suud belated a comment, sorry :) Note collect takes only elements that match a defined case; as a partial function it defines, it needs not all cases be covered, namely for instance case _ may be ommitted.

dk14 Over a year ago

@suud, enzyme no probs :)

mohit · Accepted Answer · 2015-01-16 18:02:08Z

2

Another approach can be

def foo(....) = 
   Map("name" -> name, "age" -> age, "hasChilds" -> hasChilds, "childs" ->  childs)
.filter(_._2 != None).mapValues(_.get)

As pointed out by @Dider, this can also be done, similar to @enzyme solution

Map("name" -> name, "age" -> age, "hasChilds" -> hasChilds, "childs" -> childs)
.collect {case (k, Some(v)) => (k,v) }

edited Jan 16, 2015 at 18:02

answered Jan 16, 2015 at 17:55

mohit

5,0091 gold badge25 silver badges39 bronze badges

3 Comments

Didier Dupont Over a year ago

or instead of filter + mapValues, do collect {case (k, Some(v)) => (k,v) }. Be careful with mapValues, it builds a view

null Over a year ago

What does a view mean from the mapValues?

mohit Over a year ago

@suud - As much as I know, a view is non-strict evaluation of a collection. Basically, it means that mapped values are not evaluated/transformed till they are accessed. This may improve the performance, if not all values are going to be accessed.

Daniel Fennelly · Accepted Answer · 2015-01-16 18:50:10Z

Scala generally favors typed data and immutability, and you're fighting against both of those here. I don't know what the context is for this map, but I think it would be more idiomatic to use a case clase with optional parameters. For example:

case class Person(name: String, age: Option[Int], children: Seq[Person]) {
  def hasChildren: Boolean = !children.isEmpty
}

Now you might call this as follows with an optional name variable.

val name: Option[String] = Option("suud")
val age: Option[Int] = Option(25)
val children: Seq[Person] = Seq.empty
val suud: Option[Person] = name map {Person(_, age, children)}

The way that foo is written, it's possible to pass in an empty list of children with a boolean parameter which says the map has children. Writing hasChildren as a method of a case class guards against this, because the boolean method depends on the collection it's meant to provide information about.

If you really insist on using a map here, you can either use a MapBuilder to get an immutable map, or just import and use the mutable map.

import scala.collection.mutable.MapBuilder

val builder: MapBuilder[String, Any, Map[String, Any]] = new MapBuilder(Map.empty)

if (!name.isEmpty) builder += ("name" -> name.get)
if (!age.isEmpty) builder += ("age" -> age.get)
if (!hasChilds.isEmpty) builder += ("hasChilds" -> hasChilds.get)
if (!childs.isEmpty) builder += ("childs" -> childs.get)

builder.result

Now the result of the builder is an immutable map. If you really need a mutable map, you can just:

import scala.collection.mutable

val m = mutable.Map[String, Any]()

Now you've got a mutable map which can be converted to an immutable map with its toMap method.

Collectives™ on Stack Overflow

Need help to refactor this scala method in functional programming style

3 Answers 3

4 Comments

3 Comments

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

3 Answers 3

4 Comments

3 Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Related