How can i transform this scala function in order to be optimized

Question

Code to determine the lat element of a list, using pattern matching:

  @tailrec
  def last_rec[A](list : List[A]) : A = {
    list match {
      case (x :: Nil) => x
      case (_ :: xs) => last_rec(xs)
      case Nil => throw new NoSuchElementException
    }
  }

I want to compile the code, I am getting "yelled" by the compiler:

PS D:\workspace\scala\P99> scalac .\P01.scala
.\P01.scala:18: error: could not optimize @tailrec annotated method last2: it contains a recursive call not in tail position
      case Nil => throw new NoSuchElementException
                        ^
one error found

If I remove the @tailrec annotation - the code compiles . How can I modify the code in order to do the tail rec optimization ?

I think there should be one more compiler error: `last is undefined` — Jamil, Sep 10 '11 at 10:02
I'm confused: your method isn't even *recursive*, so why would you even *expect* it to be tail-recursive? — Jörg W Mittag, Sep 10 '11 at 10:06
Also: the error message you provided doesn't match up with the code you posted. The error message refers to a line number 18, which doesn't exist and it refers to a method `last2` which doesn't exist, either. — Jörg W Mittag, Sep 10 '11 at 10:07

score 3 · Accepted Answer · answered Sep 10 '11 at 10:07

3

You got a typo their. Your method is called last_rec and you are calling last which is clearly undefined. So just rename it to last. And by the way you should return Option[A] instead of A. That way you can return None when nothing is found instead of throwing the ugly NoSuchElementException.

answered Sep 10 '11 at 10:07

agilesteel

16,775
6
44
55

IMO an `Option` or `Either` is inappropriate here and an Exception should be thrown if you try to pass the function an empty list. Imagine if the real `last`, or the `head` and `tail` operations on `List` all had to return an `Option` in case they're empty! – Luigi Plinge Sep 10 '11 at 15:50

Jamil · Answer 2 · 2011-09-10T10:18:05.870

1

After removing the typo and adding agilesteel's suggestion:

@tailrec
def last_rec[A](list : List[A]) : Option[A] = {
  list match {
   case (x :: Nil) => Some(x)     
   case Nil => None
   case (_ :: xs) => last_rec(xs)
 }
}

edited Sep 10 '11 at 10:18

answered Sep 10 '11 at 10:10

Jamil

2,150
1
19
20

1

`Option(x)` should be `Some(x)` though ;) – agilesteel Sep 10 '11 at 10:14
Well, `Option(x)` would give you `None` in case the last element is `null`. That might not be the desired behavior, but it's worth noting that it is a possible way to implement it. – Nicolas Payette Sep 12 '11 at 21:05

JaimeJorge · Answer 3 · 2011-09-10T13:19:13.220

1

In this case I would do what agilesteel suggested.

However, if you really wanted to throw an exception (in another different use case), you could do it in a statically typed way:

@tailrec
  def last_rec[A](list : List[A]) : Either[NoSuchElementException,A] = {
    list match {
      case (x :: Nil) => Right(x)
      case (_ :: xs) => last_rec(xs)
      case Nil => Left(new NoSuchElementException)
    }
  }

where later you could:

last_rec(Nil) match {
 case Right(s) => println("Got a value")
 case Left(e) => println("Got an exception")
}

edited Sep 10 '11 at 13:19

answered Sep 10 '11 at 11:01

JaimeJorge

1,885
16
15

It's conventional to have exception on the `Left` in such cases (see, e.g., `Catch.either`). – Alexey Romanov Sep 10 '11 at 11:32

How can i transform this scala function in order to be optimized

3 Answers3