Project Euler - multiples of numbers

Question

I'm trying to learn functional programming and Scala, following through Project Euler problems.

I'm solving get the sum of multiples x & y in a list. So for a list [1,2,3,4,5,6,7,8,9] get multiples of 3, 5 returns 3, 5, 6 and 9, the sum being 23.

1. I'm accepting two parameters list and multiples
2. Mapping multiples to list of functions, that test if input is a multiple
3. Filtering anything that returns true in the mapped multiples list

  def multiplesOf(list: List[Int], multiples: List[Int]): List[Int] = {
    def filterMultipleOf(x: Int) : Int => Boolean = (y: Int) => {
      y % x == 0
    }
    val multiplesFilter = multiples.map(filterMultipleOf(_))
    list.filter( int => multiplesFilter.exists(mOf => mOf(int)))
  }

scala> multiplesOf(List.range(1,10), List(3,5)).sum
res: Int = 23

My main question: Is this the Scale/FP way?

Is defining filterMultipleOf inside my function still pure? Is there a better way to express this in Scala or FP?

Answer 1

It can be a bit more concise (and perhaps more idiomatic).

def multiplesOf(list: List[Int], multiples: List[Int]): List[Int] = {
  val filt: Int => Boolean = multiples.foldLeft { _: Int => false } {
    case (f, n) => x => f(x) || x%n == 0
  }
  list.filter(filt)
}

And don't name variables the same as a type (int --> Int). It's visually confusing.

Answer 2

Yes, your function is still pure. A pure function has only two requirements:

1. The return value will always be the same given the same arguments.
2. Evaluation will produce no side effects.

Since filterMultipleOf only uses integers, the second is obviously met, no mutation is possible. The first requirement is also met. Calling filterMultipleOf(1) creates a function which is distinct from the function created by calling filterMultipleOf(2). See this question on software engineering for an alternative explanation.

Is there a better way to express this function? Yes.

1. Just because your functions are short doesn't mean stop thinking about names. x and y are about as vague as you can get. Renaming x to factor and y to multiple immediately makes the function clearer. Also, multiplesOf takes a list of factors, not a list of multiples.

   Naming variables after their type is generally a bad idea. int should be called multiple. list is a bit more difficult. In this case I would leave it as is since I can't quickly come up with a better name (numbers really doesn't help much).

2. You can pass functions around without specifying a single partial. Do multiples.map(filterMultipleOf) instead of multiples.map(filterMultipleOf(_))

3. I don't see the need for filterMultipleOf, you can just use an anonymous function without losing any meaning. factors.map(factor => (multiple: Int) => multiple % factor == 0)

4. Choosing to create a function for each factor makes your function logic take more space since you now have to call each function. I believe it is more clear to create a helper function that checks all factors:

   def multiplesOf(list: List[Int], factors: List[Int]): List[Int] = {
     val isMultiple = (multiple: Int) => factors.exists(factor => multiple % factor == 0)
     list.filter(isMultiple)
   }
   

   You could further reduce this function by providing isMultiple as a lambda, but this is really dependent on your style. I'd go for it in this case.

   def multiplesOf(list: List[Int], factors: List[Int]): List[Int] = {
     list.filter(multiple => factors.exists(factor => multiple % factor == 0))
   }
   

5. It might be worth questioning the public API for this function. As it stands, it is only useful for lists of integers. What happens if you have a list of another type - say, Person and you want all people whose age is a multiple of 3 or 5? Right now, you can't use this function.

   If the function was slightly modified to tell if a single integer is a multiple of some factor, then you can use the function.

   val multipleOf = (multiple: Int, factors: List[Int]) =>
     factors.exists(factor => multiple % factor == 0)
   
   class Person(var name: String, var age: Int) {
     override def toString: String = s"$name is $age"
   }
   
   val people = List(
     new Person("Adam", 25),
     new Person("Bob", 7),
     new Person("Cate", 12),
     new Person("Dave", 20),
     new Person("Edward", 8)
   )
   
   people
     .filter(person => multipleOf(person.age, List(3, 5)))
     .foreach(println)
   

   With this change, your original function can be trivially defined:

   val multipleOf = (multiple: Int, factors: List[Int]) =>
     factors.exists(factor => multiple % factor == 0)
   
   val multiplesOf = (list: List[Int], factors: List[Int]) =>
     list.filter(multipleOf(_, factors))