I've recently switched my assignment from Java to Scala. However, it still looks like java. For example, the function below does a search on a range tree, and inside there I do some "isInstanceOf" checks.
However - replacing them with "match" seems like it would only take up more space. Can anyone suggest some improvements on how to "scalify" this code?
def rangeSearch2D(treeRoot: Node, lower: Data2D, upper: Data2D,
visited: Visited): Seq[Data2D] = {
if (treeRoot == null) {
// return empty list
return Vector()
}
// increment visit count
if (visited != null)
visited.visit2D(treeRoot)
var results = ArrayBuffer[Data2D]()
// Find nearest common ancestor with value between lower.x and upper.x
var common: Node = commonAncestor(treeRoot, lower, upper, visited)
if (common.isInstanceOf[LeafNode]) {
return Vector(common.asInstanceOf[LeafNode].data)
}
/** Common non-leaf node, must process subtree */
/** Process left subtree */
var current = common.left
while (!current.isInstanceOf[LeafNode]) {
if (visited != null)
visited.visit2D(current)
//Find a path from current to lower.x
if (lower.x <= current.midRange) {
results.appendAll(rangeSearch1D(current.right.subTree,
lower, upper, visited))
current = current.left
} else {
current = current.right
}
}
//Check if current leaf node is in range
if (inRange(current, lower, upper)) {
results.append(current.asInstanceOf[LeafNode].data)
}
/** Process right subtree */
current = common.right
while (!current.isInstanceOf[LeafNode]) {
if (visited != null)
visited.visit2D(current)
//Find a path from current to upper.x
if (upper.x >= current.midRange) {
results.appendAll(rangeSearch1D(current.left.subTree,
lower, upper, visited))
current = current.right
} else {
current = current.left
}
}
//Check if current leaf node is in range
if (inRange(current, lower, upper)) {
results.append(current.asInstanceOf[LeafNode].data)
}
return results
}
Well, first you can get rid of null, replacing the parameters that might be null with Option. In the code, you then change
if (visited != null)
visited.visit2D(x)
with
visited foreach (_ visit2D x)
Both while loops can be replaced with recursive functions. Instead of adding the result to a mutable variable, you can pass it as an immutable accumulator parameter in the recursive function.
If Node has an extractor, you could use a case guard to make the midrange test. Doesn't add much, but is more idiomatic.
I get the feeling that both while loops can be subsumed into a single recursion, but I haven't considered the algorithm enough to decided about that. If so, you could get away with the common early return.
By the way, there's a bug there, since there might not be a common ancestor within the range.
If you love us? You can donate to us via Paypal or buy me a coffee so we can maintain and grow! Thank you!
Donate Us With