Kevin - 1 year ago 83

Scala Question

I wrote this code to find the prime numbers less than the given number i in scala.

`def findPrime(i : Int) : List[Int] = i match {`

case 2 => List(2)

case _ => {

val primeList = findPrime(i-1)

if(isPrime(i, primeList)) i :: primeList else primeList

}

}

def isPrime(num : Int, prePrimes : List[Int]) : Boolean = prePrimes.forall(num % _ != 0)

But, I got a feeling the findPrime function, especially this part:

`case _ => {`

val primeList = findPrime(i-1)

if(isPrime(i, primeList)) i :: primeList else primeList

}

is not quite in the functional style.

I am still learning functional programming. Can anyone please help me improve this code to make it more functional.

Many thanks.

Recommended for you: Get network issues from **WhatsUp Gold**. **Not end users.**

Answer Source

The style looks fine to me. Although the Sieve of Eratosthenes is a very efficient way to find prime numbers, your approach works well too, since you are only testing for division against known primes. You need to watch out however--your recursive function is not tail recursive. A tail recursive function does not modify the result of the recursive call--in your example you prepend to the result of the recursive call. This means that you will have a long call stack and so findPrime will not work for large i. Here is a tail-recursive solution.

```
def primesUnder(n: Int): List[Int] = {
require(n >= 2)
def rec(i: Int, primes: List[Int]): List[Int] = {
if (i >= n) primes
else if (prime(i, primes)) rec(i + 1, i :: primes)
else rec(i + 1, primes)
}
rec(2, List()).reverse
}
def prime(num: Int, factors: List[Int]): Boolean = factors.forall(num % _ != 0)
```

This solution isn't prettier--it's more of a detail to get your solution to work for large arguments. Since the list is built up backwards to take advantage of fast prepends, the list needs to be reversed. As an alternative, you could use an `Array`

, `Vector`

or a `ListBuffer`

to append the results. With the `Array`

, however, you would need to estimate how much memory to allocate for it. Fortunately we know that `pi(n)`

is about equal to `n / ln(n)`

so you can choose a reasonable size. `Array`

and `ListBuffer`

are also a mutable data types, which goes again your desire for functional style.

Update: to get good performance out of the Sieve of Eratosthenes I think you'll need to store data in a native array, which also goes against your desire for style in functional programming. There might be a creative functional implementation though!

Update: oops! Missed it! This approach works well too **if you only divide by primes less than the square root of the number you are testing**! I missed this, and unfortunately it's not easy to adjust my solution to do this because I'm storing the primes backwards.

Update: here's a very non-functional solution that at least only checks up to the square root.

rnative, you could use an `Array`

, `Vector`

or a `ListBuffer`

to append the results. With the `Array`

, however, you would need to estimate how much memory to allocate for it. Fortunately we know that `pi(n)`

is about equal to `n / ln(n)`

so you can choose a reasonable size. `Array`

and `ListBuffer`

are also a mutable data types, which goes again your desire for functional style.

Update: to get good performance out of the Sieve of Eratosthenes I think you'll need to store data in a native array, which also goes against your desire for style in functional programming. There might be a creative functional implementation though!

Update: oops! Missed it! This approach works well too **if you only divide by primes less than the square root of the number you are testing**! I missed this, and unfortunately it's not easy to adjust my solution to do this because I'm storing the primes backwards.

Update: here's a very non-functional solution that at least only checks up to the square root.

```
import scala.collection.mutable.ListBuffer
def primesUnder(n: Int): List[Int] = {
require(n >= 2)
val primes = ListBuffer(2)
for (i <- 3 to n) {
if (prime(i, primes.iterator)) {
primes += i
}
}
primes.toList
}
// factors must be in sorted order
def prime(num: Int, factors: Iterator[Int]): Boolean =
factors.takeWhile(_ <= math.sqrt(num).toInt) forall(num % _ != 0)
```

Or I could use `Vector`

s with my original approach. `Vector`

s are probably not the best solution because they don't have the fasted O(1) even though it's amortized O(1).