Reputation: 17
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
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:
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)
}
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
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