patternMinor
Scala heap implementation
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) {
```
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
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
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.