I have a case class that is recursive and looks like this:
case class Config(name: String, isEnabled: Boolean, elems: Map[String, MyCase])
case class MyCase(
id: String,
isActive: Boolean,
elems: Option[Map[String, MyCase]])
Where the Config
contains the id
of the MyCase
entries contained as a Map
. I have to iterate over this structure and come up with a Map
that contains the parent child relations. Say for example., if I have the Config
class represented as below (for simplicity, I have just given the id
's):
Config(Map("5" -> myCase1, "6" -> myCase2))
5 - Config
1
1
2
2
6
1
where id
's 5 and 6 are top level entries which in turn has a recursive structure. I have to now come up with a Map
that contains the parent child relationship for the id
's. So for the case above, I will expect a Map
that looks like:
Map(
"5" -> Seq("5.1"),
"5.1" -> Seq("5.1.1", "5.1.2"),
"5.1.1" -> Seq.empty[String],
"5.1.2" -> Seq.empty[String],
"6" -> Seq("6.1"),
"6.1" -> Seq.empty[String]
)
Notice how I append the parent id
to the child.
Given a Seq
of such Config
s,
val configs = Seq(Config1, Config2)
this is what I have come up with:
def relationsFor(config: Config): Map[String, Seq[String]] = {
def prepareChildren(parentId: String, acc: Map[String, Seq[String]], subElems: Map[String, MyCase]): Map[String, Seq[String]] = {
subElems.collect {
case (id, elem) if elem.isActive =>
val newAcc = acc ++ Map(parentId -> subElems.keys.map(subElemId => s"$parentId.$subElemId").toSeq)
if (elem.elems.isDefined) {
val newParentId = s"$parentId.$id"
val newAcc1 = newAcc ++ Map(s"$parentId.$id" -> elem.elems.get.keys.map(subElemId => s"$newParentId.$subElemId").toSeq)
prepareChildren(s"$parentId.$id", newAcc1, elem.elems.get)
} else {
acc ++ Map(s"$parentId.$id" -> Seq.empty[String])
}
}
}.flatten.toMap
configs.collect {
case config if config.isEnabled =>
config.elems.collect {
case (parentId, elemCase) if elemCase.elems.isDefined =>
prepareChildren(parentId, Map.empty[String, Seq[String]], elemCase.elems.get)
case (parentId, _) =>
Map(parentId -> Seq.empty[String])
}
}.flatten.flatten.toMap
}
Could this be simplified ? I have already tested this and it works as expected, but I find it a little bit hard to understand. Can this be made much more elegant? I mean can the relationsFor
method be made more elegant ?
-
\$\begingroup\$ Any suggestions? \$\endgroup\$joesan– joesan2016年07月01日 16:35:22 +00:00Commented Jul 1, 2016 at 16:35
-
1\$\begingroup\$ Welcome to Code Review! I have rolled back the last edit. Please see what you may and may not do after receiving answers . \$\endgroup\$Vogel612– Vogel6122016年07月01日 17:19:49 +00:00Commented Jul 1, 2016 at 17:19
2 Answers 2
I tried do it a different way, when you asked this question on StackOverflow.. You find my version below, first I'll try to give you some feedback on your version :
I see you regularly use
isDefined
together with pattern matching or if / else. I would replace this by matching forSome
/None
or even better with a combination ofmap
andgetOrElse
or withfold
:o.map(f).getOrElse(b) o.fold(b)(f)
These are more ideomatic ways to handle an
Option
and unwrap theOption
for you where you want to use it (callingget
on anOption
is something you should try to minimize).- It seems like you could have reused
newParentId
in the places where you wrotes"$parentId.$id"
, this would mean thatnewParentId
should be placed outside the if else. - I am not sure if you can change the signature of
MyCase
, but I am not sure what extra theOption
brings forelems
, but this may be moot if you want to differentiate betweenNone
andMap.empty[String, MyCase]]
, if you don't then no collection or an empty collections is pretty much the same.
Here's what I came up with :
def getMapWithChildIds(config: Config): Map[String, Seq[String]] = {
def mkId(prefix: String, key: String): String =
if (prefix.isEmpty) key else prefix + "." + key
def go(map: Map[String, MyCase], prefix: String): Map[String, Seq[String]] =
map.flatMap { case (key, elem) =>
val fullKey = mkId(prefix, key)
// this will handle an unactive MyCase the same way as a MyCase with no elems
Option(elem).filter(_.isActive).flatMap(_.elems).fold(
Map(fullKey -> Seq.empty[String]))(
submap => {
val directSubKeys = submap.keys.map(mkId(fullKey, _)).toList
// map entry for this key + entries for sub keys
Map(fullKey -> directSubKeys) ++ go(submap, fullKey)
})
}
go(config.elems, "")
}
Which gives for :
val noMap: Option[Map[String, MyCase]] = None
val config =
Config("a", true,
Map(
"5" -> MyCase("id?5", true,
Some(Map(
"1" -> MyCase("id?5.1", true,
Some(Map(
"1" -> MyCase("id?5.1.1", true, noMap),
"2" -> MyCase("id?5.1.2", true, noMap)
))
),
"2" -> MyCase("id?5.2", true, noMap)
))
),
"6" -> MyCase("id?6", true,
Some(Map(
"1" -> MyCase("id?6.1", true, noMap)
))
)
)
)
The following result :
val map = getMapWithChildIds(config)
// Map[String,Seq[String]] = Map(5.2 -> List(), 5.1.2 -> List(), 5 -> List(5.1, 5.2), 5.1.1 -> List(), 6 -> List(6.1), 6.1 -> List(), 5.1 -> List(5.1.1, 5.1.2))
map.toList.sortBy(_._1).foreach(println)
// (5,List(5.1, 5.2))
// (5.1,List(5.1.1, 5.1.2))
// (5.1.1,List())
// (5.1.2,List())
// (5.2,List())
// (6,List(6.1))
// (6.1,List())
-
\$\begingroup\$ You are awesome! Your other example for my other question was a lot readable. Let me see if I can further simplify my version without those if else and isDefined on the Option! \$\endgroup\$joesan– joesan2016年07月01日 16:53:44 +00:00Commented Jul 1, 2016 at 16:53
-
\$\begingroup\$ The reason why I need the extra Option for the elems is that I'm reading this case class from JSON and there might or might not be this nesting for some elements \$\endgroup\$joesan– joesan2016年07月01日 17:00:56 +00:00Commented Jul 1, 2016 at 17:00
-
\$\begingroup\$ Have a look at my EDIT. I tried to come up with a simpler version, but just not sure if it would work. I could not test it right now. Will do it perhaps later. I really appreciate your help! \$\endgroup\$joesan– joesan2016年07月01日 17:13:24 +00:00Commented Jul 1, 2016 at 17:13
Here is a much better version, but it does not give me exactly what I want:
case class Config(name: String, isEnabled: Boolean, elems: Map[String, MyCase])
case class MyCase(
id: String,
isActive: Boolean,
elems: Option[Map[String, MyCase]])
def prepareChilds(parentId: String, acc: Seq[(String, Seq[String])], elems: Seq[(String, MyCase)]): Seq[(String, Seq[String])] = {
elems.foldLeft(acc) {(newAcc, elem) =>
val (assetId, assetConfig) = elem
if (assetConfig.elems.isDefined) {
prepareChilds(
s"$parentId.$assetId",
newAcc :+ (parentId, assetConfig.elems.get.filter(_._2.isActive).keys.toSeq),
assetConfig.elems.get.filter(_._2.isActive).toSeq)
} else {
newAcc :+ (parentId, Seq.empty[String])
}
}
}
val noMap: Option[Map[String, MyCase]] = None
val config =
Config("a", true,
Map(
"5" -> MyCase("5", true,
Some(Map(
"1" -> MyCase("5.1", true,
Some(Map(
"1" -> MyCase("5.1.1", true, noMap),
"2" -> MyCase("5.1.2", true, noMap)
))
),
"2" -> MyCase("5.2", true, noMap)
))
),
"6" -> MyCase("6", true,
Some(Map(
"1" -> MyCase("6.1", true, noMap)
))
)
)
)
For the following input:
val result = prepareChilds("5", Seq.empty[(String, Seq[String])], config.elems.toSeq)
I get the following result:
List((5,ArrayBuffer(1, 2)),
(5.5,ArrayBuffer(1, 2)),
(5.5.1,List()),
(5.5.1,List()),
(5.5,List()),
(5,ArrayBuffer(1)),
(5.6,List())
)