Possibly the most egregiously non-Scala thing in your code is the if...else if...else if...else
chain. Those are a bad smell in any language, particularly OO ones or functional languages with pattern matching. Scala is both. That really has to go.
Folding is the most efficient and functional way to solve this, but that's a more advanced subject for somebody who is new to Scala and FP. Here is a simple recursive solution which illustrates the clarity of pattern matching:
def balance(chars: List[Char]): Boolean = {
def go(cs: List[Char], level: Int): Boolean = cs match {
case Nil => level == 0
case ')' :: _ if level < 1 => false
case ')' :: xs => go(xs, level - 1)
case '(' :: xs => go(xs, level + 1)
case _ :: xs => go(xs, level)
}
go(chars, 0)
}
This should show how much pattern matching simplifies a recursive procedure. Each match defines one of the possible states and the appropriate action.
Because I used pattern matching in this way
- It is clear that I have comprehensively covered the possible range of states (OK, I haven't dealt with Null but this is Scala - don't use Null).
- The corresponding actions are simple and the small differences between each one easy to see.
Compare this with the complexity, lack of clarity and fragility of your if...else if chain. It is hard to compare your different conditions, hard to see if you have been comprehensive and nothing about an if chain even compels you to be testing related conditions - you can have anything in each condition.
This is a naive example but it is a good place to start. That said, Ben's is the best answer.
There is a very small amount of code duplication in this version. The final three pattern matches do the same thing with only a minor change to the input parameters. In such a small, easy to read set of code this is really not a sin (and addressing that would make the function structure more complex and less clear). Replacing the recursion with a fold, however, would remove the duplication, because the fold would take care of the repeated application of the function,which could be reduced to a simple closure adjusting the level.
Other notes about this solution:
- It is fully tail recursive optimisable, unlike your code (again, the pattern matching makes this easy to see, where the if chain does not)
- Adding a @tailrec annotation is good practice in such Scala code
- It uses an inner function for recursion rather than exposing internal state (unlike your accepted answer).