How to write the following snippet in a correct Scala functional way?

Question

I'm very new to Scala and wanted to know if this is the correct way of writing:

def createCol(prior: List[Int], current: List[Int]): List[Int] = {
  if (prior.isEmpty) List(1) ++ current
  else if (prior.tail.isEmpty)   // begin of the block to improve
    createCol(prior.tail, current ++ List(prior.head))
  else
    createCol(prior.tail, current ++ List(prior.head + prior.tail.head))
}

The part I'm interested in is this:

if (prior.tail.isEmpty)
  createCol(prior.tail, current ++ List(prior.head))
else
  createCol(prior.tail, current ++ List(prior.head + prior.tail.head))

As I'm repeating almost the same function call createCol so I tried this instead:

val ssum = if (prior.tail.isEmpty) prior.head else prior.head + prior.tail.head
createCol(prior.tail, current ++ List(ssum))

What's the best or recommended way of doing it?

Thanks

dhg · Accepted Answer · 2012-09-26 04:43:07Z

A few points:

I changed your function to use Scala's pattern-matching framework since it greatly simplifies your code.
You shouldn't do List(x) ++ someList because this constructs a single-item list unnecessarily. You should just use the prepend method :: (or +:). Same goes for appending (:+).
If prior only has one element, you know that all the recursive call is going to do is prepend 1 to the front of current. So you can drop the recursive call from the second case.
Your method is tail-recursive, so annotate it with @tailrec.
Finally, consider using a Vector instead of List. Appending to a List is O(n) because the method must traverse all the way to the end (and then reconstruct the List from the back to the front). But both prepending and appending to a Vector are effectively O(1).

So:

@tailrec
def createCol(prior: List[Int], current: List[Int]): List[Int] = {
  prior match {
    case Nil => 1 :: current
    case head :: Nil => 1 +: current :+ head
    case head :: tail => createCol(tail, current :+ (head + tail.head))
  }
}

You could also do it in two cases, like you described in your question. But you should use the headOption method instead of explicitly doing an if/else:

@tailrec
def createCol(prior: List[Int], current: List[Int]): List[Int] = {
  prior match {
    case Nil =>
      1 :: current
    case head :: tail =>
      createCol(tail, current ++ List(head + tail.headOption.getOrElse(0)))
  }
}

huynhjl · Accepted Answer · 2012-09-26 08:01:21Z

Your function seems to be equivalent to the following:

def createCol(prior: List[Int], current: List[Int]) =
  if (prior.isEmpty) 1 :: current
  else 1 :: current ::: (prior, prior.tail :+ 0).zipped.map(_ + _)

Or with Vector which should yield better performance for prepend and append if the lists are long:

def createCol(prior: Vector[Int], current: Vector[Int]) =
  if (prior.isEmpty) 1 +: current
  else (1 +: current) ++ (prior, prior.tail :+ 0).zipped.map(_ + _)

That avoids recursion and I think it's clearer to see that current is not modified, that 1 is just prepended regardless and that prior is summed with itself with an offset of 1 with the last element by itself (summed with 0) and then appended to current.

Or even to avoid the repetition of (1 +: current):

(1 +: current) ++ (
  if (prior.isEmpty) Vector()
  else (prior, prior.tail :+ 0).zipped.map(_ + _))

Collectives™ on Stack Overflow

How to write the following snippet in a correct Scala functional way?

2 Answers 2

Comments

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Related