Rahmatullah Nikyar
Rahmatullah Nikyar

Reputation: 17

Scala unused expression error with if statement

I'm very new to Scala, and tried to write a simple Scala program that gets the maximum value. I found something weird (probably a language-specific feature). Here it is:

  def max(xs: List[Int]): Int = {
    if (xs.isEmpty) {
      throw new java.util.NoSuchElementException
    }

    def maxAux(x: List[Int], curMax: Int): Int = {
      if (x.isEmpty)  {
        curMax
      }
      if (x.head > curMax) {
        maxAux(x.tail, x.head)
      }
      else {
        maxAux(x.tail, curMax)
      }
    }
    maxAux(xs.tail, xs.head)
  }
}

For some reason, inside of maxAux function, the return of the first if statement gives me an IntelliJ warning that it is an "unused expression". Turns out it is correct because that line doesn't seem to return. To get around that issue, the second if statement in maxAux I changed to an else if, and then everything worked as intended. The other fix would be to add a return statement before curMax, but apparently using return is bad style/practice.

TL;DR: Can anyone explain why in the code above curMax doesn't return?

Upvotes: 1

Views: 643

Answers (2)

Malte Schwerhoff
Malte Schwerhoff

Reputation: 12852

In def maxAux, the control-flow can enter the first if-branch, whose body yields curMax. But that if-statement is not the last statement in maxAux, another if- or else-branch will follow, and they determine the result.

If you test your function (note that your code currently doesn't compile!), e.g. via println(max(List(1,3,2,5,0))), then you'll get a NoSuchElementException, which is a consequence of your current (incorrect) implementation.

You have various options now, e.g. the following two, which only minimally change your code:

  1. Fix the if-else cascade (which you could also rewrite into a pattern matching block):

    if (x.isEmpty)  {
      curMax
    } else if (x.head > curMax) {
      maxAux(x.tail, x.head)
    } else {
     maxAux(x.tail, curMax)
    }
    
  2. Use a return statement (though you should be careful with the use of return in Scala. See the comments under this answer):

    if (x.isEmpty)  {
      return curMax
    }
    

Upvotes: 1

Tim
Tim

Reputation: 27356

The immediate problem is that Scala returns the value of the last expression in a block which is the second if/else expression. So the value of the first if is just discarded. This can be fixed by making the second if an else if so that it is a single expression.

A better solution is to use match, which is the standard way of unpicking a List:

def maxAux(x: List[Int], curMax: Int): Int =
  x match {
    case Nil =>
      curMax
    case max :: tail if max > curMax =>
      maxAux(tail, max)
    case _ :: tail =>
      maxAux(tail, curMax)
  }

Upvotes: 3

Related Questions