balanced parantheses recursive method in scala

Question

I am working through functional programming in scala and am a bit stumped on trying to write a recursive method to find balanced parentheses.

DISCLAIMER - I am not looking for a copy/paste answer, I am just hoping someone can point out why my conditional statements are not evaluating (I am very new to scala so the idea that conditional are themselves expressions is a new concept for me).

Here is my attempt so far:

  def balance(chars: List[Char]): Boolean = {
    val charStack = new Stack[Char]()

    def balanceRec(chars: List[Char]):  Boolean = {
      if (chars.isEmpty) charStack.isEmpty

      println(chars.mkString(", "))
      if (chars.head == ')' &&
          charStack.isEmpty
        ) {
          false
      }
      else if (chars.head == ')' &&
        charStack.pop() != '('
      )  {
        false
      }

      else{
        if(chars.head == '(') charStack.push(chars.head)
        balanceRec(chars.tail)
      }


    }
    balanceRec(chars)

  }

I am testing on a very simple input like val chars = List('(', 't', 'e', 's', 't', ')', 'a') and even still get a java.util.NoSuchElementException: head of empty list.

I am confused on why my second else if statement is not firing to return false. Also, I have fleshed out these conditions to ensure I understand they work as I think they do - please don't make fun of me :)

Answer 1

Let's start by cleaning up a little to make it easier what is going on.

The error is clearly within balanceRec, so we don't even need to look at balance:

def balanceRec(chars: List[Char]):  Boolean = {
  if (chars.isEmpty) charStack.isEmpty

  println(chars.mkString(", "))
  if (chars.head == ')' &&
      charStack.isEmpty
    ) {
      false
  }
  else if (chars.head == ')' &&
    charStack.pop() != '('
  )  {
    false
  }

  else{
    if(chars.head == '(') charStack.push(chars.head)
    balanceRec(chars.tail)
  }


}

Next, the println clearly is not part of the problem either, so let's remove that:

def balanceRec(chars: List[Char]):  Boolean = {
  if (chars.isEmpty) charStack.isEmpty

  if (chars.head == ')' &&
      charStack.isEmpty
    ) {
      false
  }
  else if (chars.head == ')' &&
    charStack.pop() != '('
  )  {
    false
  }

  else{
    if(chars.head == '(') charStack.push(chars.head)
    balanceRec(chars.tail)
  }


}

Further, let's remove some unnecessary parentheses and clean up the white space and indentation:

def balanceRec(chars: List[Char]): Boolean = {
  if (chars.isEmpty) charStack.isEmpty

  if (chars.head == ')' && charStack.isEmpty) false
  else if (chars.head == ')' && charStack.pop() != '(') false
  else {
    if (chars.head == '(') charStack.push(chars.head)
    balanceRec(chars.tail)
  }
}

Now the structure of the method becomes much clearer.

We notice now, that the first expression is a NO-OP: it has no side-effects, it doesn't mutate anything, it doesn't print anything, its result is not returned, its result is not printed, its result is not assigned to a variable or field. It literally does nothing at all.

That's the main problem.

As it turns out, all we need to do to fix the problem is insert one single else keyword before the second if:

def balanceRec(chars: List[Char]): Boolean = {
  if (chars.isEmpty) charStack.isEmpty
  else if (chars.head == ')' && charStack.isEmpty) false
  else if (chars.head == ')' && charStack.pop() != '(') false
  else {
    if (chars.head == '(') charStack.push(chars.head)
    balanceRec(chars.tail)
  }
}

[Scastie link]

And since the body of our method is now a single expression, we can get rid of the curly braces:

def balanceRec(chars: List[Char]): Boolean =
  if (chars.isEmpty) charStack.isEmpty
  else if (chars.head == ')' && charStack.isEmpty) false
  else if (chars.head == ')' && charStack.pop() != '(') false
  else {
    if (chars.head == '(') charStack.push(chars.head)
    balanceRec(chars.tail)
  }

Finally, some general tips.

You are not actually using any of the specific features of List in your code, you are only using the more general features of Seq, in fact, you are actually only using features of Iterable. I prefer to keep my interfaces as generic and as general as possible, so I would change the type here to Iterable:

def balance(chars: Iterable[Char]): Boolean = {
  val charStack = new Stack[Char]()

  def balanceRec(chars: Iterable[Char]): Boolean =
    if (chars.isEmpty) charStack.isEmpty
    else if (chars.head == ')' && charStack.isEmpty) false
    else if (chars.head == ')' && charStack.pop() != '(') false
    else {
      if (chars.head == '(') charStack.push(chars.head)
      balanceRec(chars.tail)
    }

  balanceRec(chars)
}

Next, I find the method awkward to use, because I have to pass in a collection of individual characters, when I would find it more natural to pass in a string:

def balance(s: String): Boolean = {
  val charStack = new Stack[Char]()

  def balanceRec(chars: Iterable[Char]): Boolean =
    if (chars.isEmpty) charStack.isEmpty
    else if (chars.head == ')' && charStack.isEmpty) false
    else if (chars.head == ')' && charStack.pop() != '(') false
    else {
      if (chars.head == '(') charStack.push(chars.head)
      balanceRec(chars.tail)
    }

  balanceRec(s.toIterable)
}

But actually, String supports head and tail as well, so there is no need to use the Iterable here:

def balance(s: String): Boolean = {
  val charStack = new Stack[Char]()

  def balanceRec(s: String): Boolean =
    if (s.isEmpty) charStack.isEmpty
    else if (s.head == ')' && charStack.isEmpty) false
    else if (s.head == ')' && charStack.pop() != '(') false
    else {
      if (s.head == '(') charStack.push(s.head)
      balanceRec(s.tail)
    }

  balanceRec(s)
}

I also prefer to use factory methods over constructors, so I would initialize the stack like this:

val charStack = Stack.empty[Char]

In functional programming, unlike many other communities, short variable names are perfectly acceptable, as long as the code still stays readable. Also, we generally trust our types, so for example, you don't have to say charStack: we know it's a stack, because it has type Stack! A typical naming pattern for naming a collection of things is to just append an s to denote the plural, so we could denote this chars or even just cs (which, as a happy coincidence, could alternatively be read as an abbreviation of "char stack").

def balance(s: String): Boolean = {
  val cs = Stack.empty[Char]

  def balanceRec(s: String): Boolean =
    if (s.isEmpty) cs.isEmpty
    else if (s.head == ')' && cs.isEmpty) false
    else if (s.head == ')' && cs.pop() != '(') false
    else {
      if (s.head == '(') cs.push(s.head)
      balanceRec(s.tail)
    }

  balanceRec(s)
}

Now that we have shortened the code quite a bit, we notice that the second and third alternative (the first two elses) have the same consequence: they both evaluate to false. So, we can simply connect their conditions with an OR:

def balance(s: String): Boolean = {
  val cs = Stack.empty[Char]

  def balanceRec(s: String): Boolean =
    if (s.isEmpty) cs.isEmpty
    else if (s.head == ')' && cs.isEmpty || s.head == ')' && cs.pop() != '(') false
    else {
      if (s.head == '(') cs.push(s.head)
      balanceRec(s.tail)
    }

  balanceRec(s)
}

Now we can apply some Boolean algebra and factor out the s.head =='(' condition:

def balance(s: String): Boolean = {
  val cs = collection.mutable.Stack.empty[Char]

  def balanceRec(s: String): Boolean =
    if (s.isEmpty) cs.isEmpty
    else if (s.head == ')' && (cs.isEmpty || cs.pop() != '(')) false
    else {
      if (s.head == '(') cs.push(s.head)
      balanceRec(s.tail)
    }

  balanceRec(s)
}

Lastly, there is no need to have a mutable accumulator to pass information between different recursive calls, because the language already gives us tools that allow us to pass information into and out of calls: arguments and return values!

So, instead of keeping our state information in a mutable stack, we can simply pass an immutable stack (and we are simply going to use a List for this) along as an argument and return it as a value:

def balance(s: String): Boolean = {
  def balanceRec(s: String, cs: List[Char] = Nil): (Boolean, List[Char]) =
    if (s.isEmpty) (cs.isEmpty, cs)
    else if (s.head == ')' && cs.isEmpty || s.head == ')' && cs.head != '(') (false, cs)
    else balanceRec(s.tail, if (s.head == '(') s.head :: cs else cs)

  balanceRec(s)._1
}

Note that unlike what I said above, we are using List[Char] here instead of Seq[Char], Iterable[Char], or String, because in this case we do make use of one of the features of List, namely O(1) prepend, head, and tail, which neither of the other three guarantee.

!!! Also note that this code is WRONG !!!

We did not account for the fact that your original code used side-effects, more precisely, mutation. For example, the second branch, even if it is not taken, may still lead to pop()ping the top element of the stack. Let me repeat that: a branch that is not taken nevertheless changes the result.

In particular, if s.head == ')' is true and cs.isEmpty is false and cs.head != '(' is false (in other words, cs.head == '(' is true), then we pop() the top-most element, and even though we are not taking this branch, this mutation is still visible in the later branches of the if / else chain as well as future recursive calls.

So, we would have to account for that, in similar way as we accounted in the final else:

def balance(s: String): Boolean = {
  def balanceRec(s: String, cs: List[Char] = Nil): (Boolean, List[Char]) =
    if (s.isEmpty) (cs.isEmpty, cs)
    else if (s.head == ')' && cs.isEmpty) (false, cs)
    else if (s.head == ')' && cs.head != '(') (false, cs.tail)
    else if (s.head == ')' && cs.head == '(') balanceRec(s.tail, if (s.head == '(') s.head :: cs.tail else cs.tail)
    else balanceRec(s.tail, if (s.head == '(') s.head :: cs else cs)
  balanceRec(s)._1
}

Now, you might look at this, and say: "I thought functional programming is supposed to be so simple and so beautiful, but all I see here is that when we converted the mutable solution to an immutable one, it became a convoluted mess of triply-nested logic". But here's the thing: the mutable version had exactly that same mess! You were just not seeing it, because the dependencies between the different parts of the code that were connected by the mutable stack, were invisible. But they were still there.

As an alternative solution, note that we do not actually need to know the content of the stack at all. We only need to know how deep the stack is! So, we could instead do something like this:

def balance(s: String): Boolean = {
  def balanceRec(cs: List[Char], depth: Int = 0): Int =
    if (depth < 0) depth
    else cs match {
      case Nil       => depth
      case '(' :: cs => balanceRec(cs, depth + 1)
      case ')' :: cs => balanceRec(cs, depth - 1)
      case c :: cs   => balanceRec(cs, depth)
    }

  balanceRec(s.toList) == 0
}

Answer 2

I would just like to point out that recursion is not always needed, and sometimes recursion is not the most functional way to do something.

If all we desire is to determine whether or not parentheses are balanced, we simply want to know if there are the same number of ( and ).

EDIT: This forgets to take into account that )( would return true, although it is invalid! I have updated the response to account for this.

To address this, we can take into account that the running total must always be greater than zero

We can use map to transform the list of Chars into a list of Ints

We can transform a ( into a +1, a ) into a -1, and all other characters we don't care about and can turn them to 0

To check for balance, the sum of our new list should be 0, and the value should never fall below zero

To achieve this, we can use the foldLeft operator, which takes two parameter lists

val testList: List[Char] = List('(', 't', 'e', 's', 't', 'i', 'n', 'g', ')', '!')
    
val mappedList: List[Int] = testList.map{ currentChar =>
    if (currentChar == '(') 1
    else if (currentChar == ')') -1
    else 0
}
    

def foldOp(currentTotal: Int, newValue: Int) = {
    if (currentTotal < 0) {
        currentTotal
    } else {
        currentTotal + newValue
    }
}

val initialValue: Int = 0

val sumResult: Int = mappedList.foldLeft(initialValue)(foldOp)
    
val isBalanced: Boolean = sumResult == 0
    // true

As a function this might look something like:

def isBalanced( charList: List[Char] ): Boolean = {
    def mapChar( char: Char): Int = {
        if (char == '(') 1
        else if (char == ')') -1
        else 0
    }
    def foldOp(currentTotal: Int, newValue: Int) = {
        if (currentTotal < 0) {
            currentTotal
        } else {
            currentTotal + newValue
        }
    }
        
    val charToIntList: List[Int] = charList.map(mapChar)
    val sum: Int = charToIntList.foldLeft(0)(foldOp)
    sum == 0
}

Pattern-matching is also a great functional tool:

def isBalanced( charList: List[Char] ): Boolean = {
    charList.map{
        case '(' => 1
        case ')' => -1
        case _ => 0
    }.foldLeft(0)(foldOp) == 0
}

My goal here is not to provide you with an answer that you should copy/paste, rather, I would like to share my own approach and interpretation of how to express a solution to such a problem. My goal is to hopefully inspire a solution or to give a peek into the many different ways to solve a problem.

Now, that being said, this can absolutely be done recursively:

def isBalanced( charList: List[Char], runningTotal: Int = 0 ): Boolean = {
    if (runningTotal < 0) {
        false
    }
    charList match {
            // Base Case (1) Empty List, return whether or not the total is 0
        case Nil => runningTotal == 0
            // Recursive Case head char and the rest of the list
        case headChar :: rest =>
            val mappedValue: Int = headChar match {
                case '(' => 1
                case ')' => -1
                case _ => 0
            }
            isBalanced(rest, runningTotal + mappedValue)
    }
}

Finally, I would also like to point out filter. For small lists, this is a rather verbose way of doing things, but if the list is reaaaaaaaally big, this will cut the size of the elements to check (perhaps better suited to a more complex use-case)

As Luis pointed out, collect is way better for this, it will apply the partial function (matching) [syntactic sugar for _ match { case ... }] like map, but for any case that isn't defined, it will not be collected.

def isBalanced( charList: List[Char] ): Boolean = {
    val mappedValues: List[Int] = charList.collect{
        case '(' => 1
        case ')' => -1
    }
    mappedValues.foldLeft(initialValue)(foldOp)
}