curiousengineer curiousengineer - 2 months ago 12
Scala Question

Getting max from a case class List of nodes, returning Scala Option[List]

I have a case class Node that I have written, and create a list from it and I need to find the node that has maximum disk.
I wrote the below code, is there a better way of doing it? Also, in my actual production code, my "nodeList" variable will be not just

Option[List[Node]]
but
Future[Option[List[Node]]]
. I guess still the answer/code won't change much except for the fact that I will do a map/flatMap to go inside the future and do the same thing.
If anyone has a better suggestion to write below code more Scala way, please share your thoughts.

scala> case class Node(disk: Integer, name: String)


defined class Node

scala> val nodeList = Option(List(Node(40, "node1"), Node(200, "node3"),Node(60, "node2")))
nodeList: Option[List[Node]] = Some(List(Node(40,node1), Node(200,node3), Node(60,node2)))

scala> val maxDisk = nodeList match {
| case None => println("List is empty"); None
| case Some(lst) => {
| Some(lst.max(Ordering.by((_:Node).disk)))
| }
| }`
maxDisk: Option[Node] = Some(Node(200,node3))

Answer Source

Judging by the code you wrote, I'm not sure if you really should use Optional[List[Node]]. You seem to treat None as an empty List, and you don't check for the empty list in the Some case. You might want to see if just a plain List[Node] suits your use better (where None would become Nil, and Some(lst) is just lst, and the unused Some(Nil) case no longer exists to confuse anyone).

If you do keep Optional[List[Node]], I'd do it like this:

nodeList
  .filterNot(_.isEmpty) // maxBy throws if the list is empty; check for it
  .map(_.maxBy(_.disk)) // maxBy looks nicer than max(Ordering.by)

If you switch to List[Node], it's slightly uglier:

Some(nodeList)
  .filterNot(_.isEmpty) // We're using the filter utility of Option here,
  .map(_.maxBy(_.disk)) // so I wrap with Some to get access to filterNot.