How do I determine if an expression has balanced brackets in a stack?

Question

So I wrote this code to determine if an expression has balanced brackets in a stack:

public static boolean isBalanced(String expr) {
    StringStack stack = new StringStackRefBased();
    try{
    for (int i = 0; i<expr.length(); i++){
        if (expr.charAt(i) == ('(')){
            stack.push("(");
        } else if (expr.charAt(i) == (')')){
            stack.pop();
        }
    }
    if (stack.isEmpty()){
        return true;
        } else {
            return false;
        }
    } catch (StringStackException e) {
        return false;
    }
}

The problem is that the stack keeps on returning false even if the expression has balanced parentheses so what's wrong with my code?
Here's the code for StringStackRefBased

public class StringStackRefBased implements StringStack {
    private StringNode head;

    public boolean isEmpty(){
        return head == null;
    }

    public void push(String item) throws StringStackException{
        head = new StringNode(item);
    }

    public String pop() throws StringStackException{
        String result = null;
        if(isEmpty()){
            throw new StringStackException("Empty Stack");
        }
        head.next = head;
        return head.toString();
    }

    public String peek() throws StringStackException{
        if (isEmpty()){
            throw new StringStackException("Stack underflow");
        }
        return head.toString();
    }
}

Well StringStack doesn't have much in it but StringStackRefBased has all the methods in it. I've updated the code to show StringStackRefBased. The biggest thing I'm wondering is, is the push and pop methods correct? — Brandon MacLeod, Mar 18 '15 at 20:47
Why not just have an int and do `count++` and `count--`? Then at the end you can check if the count is zero. — Bubletan, Mar 18 '15 at 20:47
The problem lies in your `StringStack` because if I replace it with Java's built-in `Stack`, it works just fine. — Bart Kiers, Mar 18 '15 at 20:49
verify the push method. I think the problem is there itself. — Aditya Peshave, Mar 18 '15 at 20:49
I've updated it with StringStackRefBased because StringStack has the empty constructors of StringStackRefBased. — Brandon MacLeod, Mar 18 '15 at 20:50
your implementation of `isEmpty()` is incompatible with your implementation of `push()` and `pop()`. After calling `push()`, head is never `null` no matter how many times you've called `pop` — Ian McLaird, Mar 18 '15 at 20:52
As @GriffeyDog says, both `push` and `pop` have issues. But critically, `pop` doesn't actually remove anything from the stack as it's supposed to. `push` just replaces `head` but doesn't make the stack any deeper. — Ian McLaird, Mar 18 '15 at 20:55
first of all, `StringStack` is deprecated. JAVADOC: This class is not a Stack, it is a String utility. As such it is deprecated in favour of the StringUtils class in the [lang] project. So you have to make sure you implement all the methods including `isEmpty()` as @IanMcLaird said — Aditya Peshave, Mar 18 '15 at 20:55
But is it possible to write push and pop without declaring any new variables or do I need to declare a size variable to keep track of how big the stack is. — Brandon MacLeod, Mar 18 '15 at 20:58

Bart Kiers · Accepted Answer · 2015-03-18T21:04:56.200

The method is fine. If I use Java's own stack:

class Main {

  public static boolean isBalanced(String expr) {
    Stack<String> stack = new Stack<>();
    try{
      for (int i = 0; i<expr.length(); i++){
        if (expr.charAt(i) == ('(')){
          stack.push("(");
        } else if (expr.charAt(i) == (')')){
          stack.pop();
        }
      }
      if (stack.isEmpty()){
        return true;
      } else {
        return false;
      }
    } catch (Exception e) {
      return false;
    }
  }

  public static void main(String[] args) {
    System.out.println(isBalanced("("));
    System.out.println(isBalanced("(()"));
    System.out.println(isBalanced("())"));
    System.out.println(isBalanced("((()))"));
    System.out.println(isBalanced("(()())"));
  }
}

will print:

false
false
false
true
true

Btw, your return statement is rather verbose, and using an exception that way is bad practice. Exceptions are just that: an exception(al case). This is IMO better:

public static boolean isBalanced(String expr) {
  Stack<String> stack = new Stack<>();

  for (int i = 0; i < expr.length(); i++) {
    if (expr.charAt(i) == ('(')){
      stack.push("(");
    }
    else if (expr.charAt(i) == (')')) {
      if (stack.isEmpty()) {
        return false;
      }
      stack.pop();
    }
  }

  return stack.isEmpty();
}

This is how your stack would work properly:

class StringStack {

  private StringNode head = null;

  public boolean isEmpty(){
    return head == null;
  }

  public void push(String item) {
    StringNode oldHead = head;
    head = new StringNode(item);
    head.next = oldHead;
  }

  public String pop() throws StringStackException {
    if (isEmpty()) {
      throw new StringStackException("Empty Stack");
    }
    String result = head.item;
    head = head.next;
    return result;
  }

  public String peek() throws StringStackException {
    if (isEmpty()) {
      throw new StringStackException("Stack underflow");
    }
    return head.item;
  }

  static class StringNode {

    String item;
    StringNode next;

    public StringNode(String item) {
      this.item = item;
    }
  }
}

@GriffeyDog, the question is about his method (the logic behind it), which is just fine. I just showed him he needs to look at his stack. If he would provide the (complete) source for his stack, I am happy to provide info in my answer on it. — Bart Kiers, Mar 18 '15 at 20:55
Yes, I know. Again, if he'd provide the complete source of that class, I am more than happy to edit my answer with extra information (if someone esle doesn't do so before me). — Bart Kiers, Mar 18 '15 at 20:58
First off the problem is my push and pop methods don't work correctly and I'm not sure how to fix them. For push I'm trying to set the old head to the previous node and keep on increasing the size of the stack. — Brandon MacLeod, Mar 18 '15 at 21:02

How do I determine if an expression has balanced brackets in a stack?

1 Answers1