Perfect Number usage in higher order function

Question

I am new to Scala.

I have been trying to complete an exercise where the ask was as follows:

// Write a function isPerfectNumber which takes integer input and returns String output.

// It finds if a number is perfect, and returns true if perfect, else returns false

// Write a higher order function myHigherOrderFunction which takes isPerfectNumber and intList as input, and returns a List of Strings which contain the output if the number is perfect or not using map.

Perfect Number :

https://rosettacode.org/wiki/Perfect_numbers#Scala

My Code :

object ListMapHigherOrder{
def main(args:Array[String])
{

val intRes = args.toList
val intList: List[Int] = intRes.map(_.toInt).toList

 def isPerfectNumber(input: Int) :String  = 
 {
 var check_sum = ( (2 to math.sqrt(input).toInt).collect  { case x if input % x == 0 => x + input / x}  ).sum 
 if ( check_sum == input - 1 )
      return "true"
      else 
      return "false"
      }

def myHigherOrderFunction(argFn: Int => String, argVal:List[Int]): List[String] = { argVal.map(argFn) }

println(myHigherOrderFunction(isPerfectNumber, intList))

}
}

Code execution : scala ScalaExcercise12.scala 1 6 13

Expected Output : List(false , true , false)

the code gives expected output, am not sure how the backend testing is being done.... it just dosent pass the test.

Is there any issue with the code? - i did like to fix it , but cant i see anything wrong/missing

any help or direction is greatly appreciated.

Answer 1

Here is my corrected solution.

object ListMapHigherOrder{
  def main(args:Array[String]) = {

    val intList: List[Int] = args.map(_.toInt).toList

    def isPerfectNumber(input: Int) :String  = {
      val check_sum = ( (2 until input).collect  { case x if input % x == 0 => x }  ).sum
      if (input <= 1) "false"
      else if ( check_sum == input - 1 ) "true"
      else "false"
    }

    def myHigherOrderFunction(argFn: Int => String, argVal:List[Int]): List[String] = { argVal.map(argFn) }

    println(myHigherOrderFunction(isPerfectNumber, intList))
  }
}

Since this is supposed to be code review, I will not mince words. Some problems I noticed with your code:

1) Your formatting is irregular. For anyone trying to understand your code, this is a needless obstruction.

2) You do not check if the number is negative or equal to 1. In all such cases, the number is not perfect. You need to check for these cases.

3) You are using a var for check_sum. This is an eyesore for anyone who knows the language. Prefer immutable values.

4) You copied and pasted your code for isPerfectNumber from someone else without understanding what it does. In this case, the code you copied and pasted is actually wrong, because it gives the wrong answer for 1. It is probably also a good idea to explicitly check if the number is negative. The original code was also relatively inefficient because of the call to square root. The change to remove square root is only a micro-optimization, but it should be faster.

5) You are trusting that calling toInt on a String won't throw an exception. This depends on the assignment specification. If the exercise assumes you will always get well-formed input, then I suppose this is fine. But if not, you need to use a try-catch block or a Try monad. Since I am not sure if you need this stuff, I won't add it to your solution.

6) The idiom for myHigherOrderFunction is strange. There is almost no point to passing a list and a lambda function to a function just so you can call map on it. If you need to call map, just call map. Creating a wrapper function is unnecessary and an eyesore. I understand that this is what the assignment asks you to do, probably to test your understanding of functions as values. However, it is ugly code, I cannot help but mention that this should be avoided normally.

I think this question is rather manageable, and that if you had spent more time on it, you would have been able to get it yourself. However, I will not lecture you. The biggest mistake you made was trusting someone else's code without checking it yourself. So you should not trust me either.