Reputation: 1028
I'm trying to implement a simple binary tree using scala. My tree definition is as below:
abstract class Tree[T <: Ordered[T]]
case class Node[T <: Ordered[T]](v:T, l:Tree[T], r:Tree[T]) extends Tree[T]
case class Leaf[T <: Ordered[T]](v:T) extends Tree[T]
And I'm trying to implement a method to find the minimum value in the tree. The method implementation is as below:
def minInTree[T <: Ordered[T]](t: Tree[T]): T = t match {
case Node(v, lft, rght) if minInTree(lft) > minInTree(rght) => {
val mLft = minInTree(lft)
val mRght = minInTree(rght)
if (v > mRght)
mRght
else
v
}
case Node(v, lft, rght) if minInTree(lft) <= minInTree(rght) => {
val mLft = minInTree(lft)
val mRght = minInTree(rght)
if (v > mLft)
mLft
else
v
}
case Leaf(lf) => lf
}
I found that minInTree(lft)
and minInTree(rght)
are called multiple times, which I believe is slow and ugly. The comparison uses multiple if-else
clause, which I believe is also ugly.
Is there any elegant way to restructuring the code? Any hints are appreciated.
Upvotes: 1
Views: 428
Reputation: 938
As jwvh wrote, you can use Ordering to have a cleaner code. An other way to have a cleaner code is to use inheritence in place of pattern matching :
println("Welcome to the Scala worksheet") //> Welcome to the Scala worksheet
sealed trait Tree[T] {
def min(implicit ev : Ordering[T]) : T
}
case class Node[T](value : T, left : Tree[T], right : Tree[T]) extends Tree[T] {
def min(implicit ev : Ordering[T]) : T = ev.min(value, ev.min(left.min(ev), right.min(ev)))
}
case class Leaf[T](value : T) extends Tree[T] {
def min(implicit ev : Ordering[T]) : T = value
}
val tree = Node(17, Leaf(2), Node(4, Leaf(10), Leaf(8)))
//> tree : testTree#626615.Node#1020777[Int#918] = Node(17,Leaf(2),Node(4,Leaf(
//| 10),Leaf(8)))
tree.min //> res0: Int#918 = 2
I prefer to add the Ordering typeclass only for the min method, when you need it. If you still want a function, it is very easy :
def minTree[T](t : Tree[T])(implicit ev : Ordering[T]) = t.min
minTree(tree) //> res1: Int#918 = 2
Upvotes: 2
Reputation: 51271
If you were to use the Ordering
type class you could then employ the min()
method on your values, which makes things much more elegant.
abstract class Tree[T: Ordering]
case class Node[T: Ordering](v:T, l:Tree[T], r:Tree[T]) extends Tree[T]
case class Leaf[T: Ordering](v:T) extends Tree[T]
def minInTree[T](t: Tree[T])(implicit ev:Ordering[T]): T = t match {
case Node(v, lft, rght) => ev.min(v, ev.min(minInTree(lft), minInTree(rght)))
case Leaf(lf) => lf
}
Import some handy implicits and the min
evaluations are even clearer.
import Ordering.Implicits._
def minInTree[T: Ordering](t: Tree[T]): T = t match {
case Node(v, lft, rght) => v min minInTree(lft) min minInTree(rght)
case Leaf(lf) => lf
}
It also makes the basic comparison operations available. (<
, >
, etc.)
Upvotes: 3
Reputation: 170835
I would just move the comparison later, I don't think there's a good way to achieve this otherwise:
case Node(v, lft, rght) => {
val mLft = minInTree(lft)
val mRght = minInTree(rght)
if (mLft > mRght) ... // corresponds to if minInTree(lft) > minInTree(rght) => ...
else ... // corresponds to your second branch
}
Upvotes: 2