patternjavascriptMinor
Checking if parentheses are balanced
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(')(')); //falseSolution
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
-
If you do the above, you don't need
-
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.