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

Scala heap implementation

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

Problem

I'm a Scala beginner, so it'd be great if anyone would be so kind to give some feedback.

```
package heap

import scala.collection.mutable.ListBuffer

class HeapT => Int) {

private val HIGHER = 1
private val LOWER = -1

private val heapRepresentation = new ListBuffer[T]()

private def lastIndex = heapRepresentation.size - 1

private def valueOf(index: Int): T = heapRepresentation(index)

def add(item: T): Heap[T] = {
heapRepresentation.append(item)
bubbleUp(lastIndex)
this
}

def swap(index1: Int, index2: Int) {
val temp = valueOf(index1)
heapRepresentation.update(index1, valueOf(index2))
heapRepresentation.update(index2, temp)
}

def bubbleUp(currentIndex: Int) {
def getParent(i: Int) = (i - 1) / 2

if (currentIndex > 0) {
val parentIndex = getParent(currentIndex)

compareFunction(valueOf(currentIndex), valueOf(parentIndex)) match {
case LOWER =>
swap(currentIndex, parentIndex)
bubbleUp(parentIndex)
case _ =>
}
}
}

def bubbleDown(currentIndex: Int) {
getLowerChild(currentIndex) match {
case Some((lowerChildIndex, lowerChildValue)) =>
if (compareFunction(valueOf(currentIndex), lowerChildValue) == HIGHER) {
swap(currentIndex, lowerChildIndex)
bubbleDown(lowerChildIndex)
}
case None =>
}
}

def getLowerChild(index: Int): Option[(Int, T)] = {
def getChildrenIndices(parentIndex: Int): (Int, Int) = (2 parentIndex + 1, 2 parentIndex + 2)

val (leftChildIndex, rightChildIndex) = getChildrenIndices(index)

val areChildrenInBoundsOfHeap = (rightChildIndex Some((leftChildIndex, leftChildValue))
case _ => Some((rightChildIndex, rightChildValue))
}
}

def pop: Option[T] = heapRepresentation.size match {
case 0 => None
case _ =>

val firstValue = heapRepresentation(0)
if (heapRepresentation.size == 1) {

Solution

Writing utility classes is hard enough without reinventing everything from scratch each time. Whenever possible, class authors should take advantage of available tools. In this case, Scala has a trait for ordering objects in a collection: scala.math.Ordering and it should be used for the compareFunction which will undoubtedly require some adjustment of the code.

For the most part, the code is well laid out and easy to follow. Good job! Note that standard indentation for Scala code is two spaces. It's hard to tell if the four space indentation is what is used in the code, or if it was an artifact of transferring the code into this question.

While Scala pattern matching is a great tool, it is possible to over user it. I think that using it in the pop method is not just overkill, but also results in code that is harder to read. I would write it like this:

def pop: Option[T] =  match {
  if (heapRepresentation.isEmpty) None
  else {
    val firstValue = heapRepresentation(0)
    if (heapRepresentation.size == 1) {
       heapRepresentation.remove(0)
    }
    else {
      swap(0, lastIndex)
      heapRepresentation.remove(lastIndex)
      bubbleDown(0)
    }
    Some(firstValue)
  }
}

Code Snippets

def pop: Option[T] =  match {
  if (heapRepresentation.isEmpty) None
  else {
    val firstValue = heapRepresentation(0)
    if (heapRepresentation.size == 1) {
       heapRepresentation.remove(0)
    }
    else {
      swap(0, lastIndex)
      heapRepresentation.remove(lastIndex)
      bubbleDown(0)
    }
    Some(firstValue)
  }
}

Context

StackExchange Code Review Q#54108, answer score: 5

Revisions (0)

No revisions yet.