HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavascriptMinor

Checking if parentheses are balanced

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
arebalancedcheckingparentheses

Problem

This script checks if parentheses are balanced. I wonder if there is something that can be improved here including ES6 features.

function isBalanced(str) {
    var stack = [];
    const allowedSymbols = '()[]<>';

    for(let i = 0, l = str.length; i < l; i++) {
        var c = str[i];
        var isOpen;
        var symbolPosition = allowedSymbols.indexOf(c);

        if(!~symbolPosition) {
            continue;
        }

        isOpen = symbolPosition % 2 ? false : true;

        if(isOpen) {
            stack.push(c);
        } else {
            if(stack.length < 1 || allowedSymbols.indexOf(stack.pop()) !== symbolPosition - 1) {
                return false;
            }
        }
    }

    return stack.length < 1;
}

console.log('()', isBalanced('()')); //true
console.log(')(', isBalanced(')(')); //false

Solution

I linked to a very related question in a comment, and this will repeat some points from my answer to that other question. Still, there are some other things here.

-
Don't do stack.push(c). Do stack.push(symbolPosition + 1) instead. I.e. push the expected position, not the current character. When you just push the character, you have to call indexOf again later to get that character's symbol position when you pop it off of the stack. By just pushing the position to the stack to begin with, you avoid that. And by adding 1 when pushing it, you get a cleaner conditional, since you can just do stack.pop() !== symbolPosition

-
If you do the above, you don't need stack.length

-
I'd skip the
isOpen variable. Sure, it might help explain what that modulo operation means, but you only need the result once. I'd rather put the explanation in a comment, and skip the single-use variable.

Similarly, I'd skip the
c variable, which - if you follow the first point above - is also only used once (and whose single-letter name doesn't help explain much), and just do indexOf(str[i]).

-
I'd change the last line to
return stack.length === 0 just to be explicit. Yes the result is the same, but the intent is clearer in my opinion. "Is the stack empty?" versus "Is the stack's length less than 1?". You could also do return !stack.length but that's unnecessarily cryptic.

In the end you get something like:

function isBalanced(str) {
    var stack = [];
    const allowedSymbols = '()[]<>';

    for(let i = 0, l = str.length; i < l; i++) {
        var symbolPosition = allowedSymbols.indexOf(str[i]);

        if(!~symbolPosition) {
            continue;
        }

        if(symbolPosition % 2 === 0) {
            stack.push(symbolPosition + 1);
        } else {
            if(stack.pop() !== symbolPosition) {
                return false;
            }
        }
    }

    return stack.length === 0;
}


As others have pointed out, you can do some tweaking with regard to
const vs let vs var`, but I'll leave that to you.

Code Snippets

function isBalanced(str) {
    var stack = [];
    const allowedSymbols = '()[]<>';

    for(let i = 0, l = str.length; i < l; i++) {
        var symbolPosition = allowedSymbols.indexOf(str[i]);

        if(!~symbolPosition) {
            continue;
        }

        if(symbolPosition % 2 === 0) {
            stack.push(symbolPosition + 1);
        } else {
            if(stack.pop() !== symbolPosition) {
                return false;
            }
        }
    }

    return stack.length === 0;
}

Context

StackExchange Code Review Q#143546, answer score: 2

Revisions (0)

No revisions yet.