Would you consider this to be idiomatic Scala? If not, what would you do to improve it?
def fibonaccis(max: Int): List[Int] = { fibonaccis(max, 2, 1) }
private def fibonaccis(max: Int, prev: Int, prevPrev: Int): List[Int] = prev >= max match {
case true => List[Int](prevPrev)
case false => prevPrev :: fibonaccis(max, prev + prevPrev, prev)
}
1 Answer 1
Naming the parameters prev
and prevPrev
is weird. Why not current
and prev
?
You shouldn't need the curly braces here for a function that consists of a single expression:
def fibonaccis(max: Int): List[Int] = fibonaccis(max, 2, 1)
Conventionally, the Fibonacci sequence is said to start like
$1,ドル 1, 2, 3, 5, 8, \ldots$$
or
$0,ドル 1, 1, 2, 3, 5, \ldots$$
Your fibonaccis(13)
would produce List(1, 2, 3, 5, 8)
, which I would consider to be missing the first element.
The Fibonacci sequence is an infinite sequence. It would be a shame to limit it to a max
value. The most idiomatic way to model it in Scala would be using an infinite lazy stream, just as suggested in the documentation:
def fibFrom(a: Int, b: Int): Stream[Int] = a #:: fibFrom(b, a + b)
val fibonaccis = fibFrom(0, 1)
val fibonaccis: Stream[Int] = 0 #:: 1 #:: fibonaccis.zip(fibonaccis.tail).map(
n => n._1 + n._2
)
Then, you could do fibonaccis.takeWhile(_ < 13).toList
to obtain List(0, 1, 1, 2, 3, 5, 8)
.
-
\$\begingroup\$ I'm not crazy about prev and prevPrev. Not current, though, since it's not the current value, it's the previous. Awkward either way. I like the stream approach - thought the answer might include that, but I haven't gotten that far yet. When is the next value generated? Is it just in time? \$\endgroup\$Don Branson– Don Branson2016年11月03日 00:34:27 +00:00Commented Nov 3, 2016 at 0:34
-
1\$\begingroup\$ Streams are lazy. Each successive element is computed as needed, and memoized for the future. \$\endgroup\$200_success– 200_success2016年11月03日 00:36:18 +00:00Commented Nov 3, 2016 at 0:36