4

I must have some basic misunderstanding of the Scala 'match' semantics or the compiler logic. This code:

val stageStart:Int = 0
val stageShutDown:Int = Int.MaxValue
val stageErrorReport:Int = Int.MinValue

def stageString(stage:Int):String = stage match {
  case stageStart       => "Start"
  case stageShutDown    => "End"
  case stageErrorReport => "Error"
  case _                => "Step " + String.valueOf(stage)
}

results in "Unreachable Code" errors on the last 3 'case' statements? If instead of the names you substitute the actual values (0, Int.MaxValue, Int.MinValue) it compiles -- but now I've hard-coded values that should be referenced by their names (for all the usual reasons). Since a 'val' can never change, shouldn't the first version also work?

  • btw intellij highlights this as warning http://gyazo.com/147de35d25071dafa8de1290b5d23e26 – OlegYch Aug 29 '12 at 19:45
  • A warning would be greatly appreciated! Eclipse raised no warning. Checked the various properties of the compiler to see if I could turn on a warning for such cases, but did not find anything. – user1624503 Aug 31 '12 at 20:43

2 Answers2

9

There is a subtle yet important feature: If the identifier in case rules start with a lower-case letter, they're always treated as variables. So the first case matches always (storing stage into variable stageStart) and the rest 3 are unreachable. You need to define the constants with upper case letters as

val StageStart:Int = 0
val StageShutDown:Int = Int.MaxValue
val StageErrorReport:Int = Int.MinValue

def stageString(stage:Int):String = stage match {
  case StageStart       => "Start"
  case StageShutDown    => "End"
  case StageErrorReport => "Error"
  case _                => "Step " + String.valueOf(stage)
}

Then they won't be treated as variables but as constants to pattern-match on.

See also this answer for Naming convention for Scala constants?

Community
  • 1
  • 1
Petr
  • 62,528
  • 13
  • 153
  • 317
8

The issue is that when you use a variable that starts with a lowercase character, the pattern matcher thinks that you are trying to assign to that variable. So you get this behavior:

val stageStart = 0
val stage = 5
def stageString(stage: Int) = stage match {
  case stageStart => println(startStage)  // prints "5"
}

Of course, a pattern that is simply an assignable variable will match anything, so any subsequent case will be unreachable.

To solve this, you need to use a "stable identifier". This can be done by putting the lowercased variable in backticks:

val stageStart = 0
def stageString(stage: Int) = stage match {
  case `stageStart` => "Start"
}

or renaming the variable so that it starts with an uppercase character:

val StageStart = 0
def stageString(stage: Int) = stage match {
  case StageStart => "Start"
}

Also: String.valueOf(stage) should be rewritten as stage.toString.

dhg
  • 52,383
  • 8
  • 123
  • 144
  • Wow! Thanks for the insight -- would have never guessed that. – user1624503 Aug 31 '12 at 19:17
  • But that brings up a more serious issue. Are you implying in the first example that a variable that was declared as a **val** can actually be **re-assigned** a different value by this match statement? That seems really scary! Or is the **case stageStart** actually considered to be a temporary variable -- which is scary in a different way. – user1624503 Aug 31 '12 at 19:22
  • A little research to answer my own question. The **case stageStart** is actually introducing a new **stageStart** variable that shadows the original. No warning issued by the Eclipse/Scala environment. Still consider this very scary -- read the book twice cover to cover, and many other sections multiple times, and the one about uppercase vs lowercase variable names went right by me. Could also see as a major problem for new Scala people, or anyone trying to migrate a bunch of legacy Java over to Scala. – user1624503 Aug 31 '12 at 21:02
  • In my opinion, one of the best syntactic features of Scala is the **override** modifier. Makes it impossible to accidentally override a method in a base class, or think that you are overriding a method but mistype the name and wind up not overriding anything. Yet now we can accidentally shadow a variable with no warning (at least in Eclipse). I'd vote for requiring the **override** modifier in this case also, to prevent the same type of errors. – user1624503 Aug 31 '12 at 21:11
  • Yep, it introduces a new variable with the same name but in a lower scope, thus shadowing the existing variable. But shadowing isn't limited to `match` statements. For example, `val x = 1; { val x = "a"; { val x = 'A; println(x) }; println(x) }; println(x)` is totally valid in Scala. – dhg Aug 31 '12 at 23:46
  • Yes, agree that it is perfectly valid in Scala -- just in my opinion this is not a good thing. I've written an awful lot of code and can't remember any case where I really wanted to re-declare and shadow a variable within a lower scope. Believe that it opens the door for nasty bugs, and really sets a trap for anyone maintaining the code a year later. Can anyone come up with a good use-case where shadowing a variable is necessry/desirable? – user1624503 Sep 03 '12 at 15:02