HiveBrain v1.2.0
Get Started
← Back to all entries
patternMinor

Idiomatic Scala Generics

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
scalaidiomaticgenerics

Problem

I'm working on some Scala problems given here, specifically:

sealed abstract class Tree[+T]
case class Node[+T](value: T, left: Tree[T], right: Tree[T]) extends Tree[T] {
  override def toString = "T(" + value.toString + " " + left.toString + " " + right.toString + ")"
}
case object End extends Tree[Nothing] {
  override def toString = "."
}
object Node {
  def apply[T](value: T): Node[T] = Node(value, End, End)
}


While implementing an insert into a height balanced tree I have the following function:

def insert(node: Tree[Char], value: Char): Tree[Char] = {
    if(node.eq(End)){
      Node(value,End,End)
    }
    else{
      val internalNode = node.asInstanceOf[Node[Char]]
      if(internalNode.left.height == internalNode.right.height){

          if(Random.nextBoolean()){ //choose left
            internalNode.left = insert(internalNode.left,value)
          }else{
            internalNode.right = insert(internalNode.right,value)
          }

      }
      else if(internalNode.left.height > internalNode.right.height){ //insert into right
          internalNode.right = insert(internalNode.right,value)
      }
      else{ //insert into left
          internalNode.left = insert(internalNode.left,value)
      }
      internalNode
    }

  }


Could you please let me know how I could improve this code.

-
Is there a way I can avoid doing the casting in asInstanceOf?

-
How can I improve the if else checks?

General feedback is also much appreciated.

Solution

When you feel yourself going to use a cast, especially when combined with an if on the save variable, then you most likely want to use a match.

def insert(node: Tree[Char], value: Char): Tree[Char] = {
  node match {
    case End => // hand end case here
    case n: Node[Char] => // handle the node case here
    case _ => // throw an exception, or do nothing if this case can't happen
  }
}


Now the node case is complicated enough that you might extract a method for it. Otherwise, it looked fine to me.

Code Snippets

def insert(node: Tree[Char], value: Char): Tree[Char] = {
  node match {
    case End => // hand end case here
    case n: Node[Char] => // handle the node case here
    case _ => // throw an exception, or do nothing if this case can't happen
  }
}

Context

StackExchange Code Review Q#59068, answer score: 2

Revisions (0)

No revisions yet.