# Balanced parenthesis string checker

Please review my balanced parenthesis string checker code below.

I would love to know how to improve my algorithm and if you have any other feedback.

// comments inline

function validateParenthesisString(inputString){

if(inputString.length === 0){ // empty string is as good as a valid string
console.log('String is valid.');
return;
}

var element,
parenthesisStack = [];

// iterate through the string
for(var index = 0, length = inputString.length; index < length; index++){

element = inputString.charAt(index); // cache the character

switch(element){
case '(':
parenthesisStack.push('('); // openeing bracket, so just push it
break;

case ')':
if(parenthesisStack[parenthesisStack.length-1] === '('){ // check if corresponding counterpart pushed already
parenthesisStack.pop(); // remove
} else {
parenthesisStack.push(')');
}
break;

case '[':
parenthesisStack.push('[');
break;

case ']':
if(parenthesisStack[parenthesisStack.length-1] === '['){
parenthesisStack.pop();
} else {
parenthesisStack.push(']');
}
break;

case '{':
parenthesisStack.push('{');
break;

case '}':
if(parenthesisStack[parenthesisStack.length-1] === '{'){
parenthesisStack.pop();
} else {
parenthesisStack.push('}');
}
break;

default: // garbage character
console.log('Invalid character encountered: ' + inputString.charAt(index));
console.log('parenthesisStack:', parenthesisStack);
console.log('***********************************');
return;
};

}

console.log('parenthesisStack:', parenthesisStack); // just for debugging

// check parenthesisStack
if(parenthesisStack.length === 0){
console.log('String is valid!');
} else {
console.log('String is invalid!');
}

console.log('***********************************');

};

validateParenthesisString('test'); // invalid
validateParenthesisString('({)}[()]{'); // invalid
validateParenthesisString('[({})]{}'); // valid

Equivalent jsFiddle.

## Major

Overall, it seems good.

I would not check for a base case as your code can deal with empty strings. It adds complexity and clutter.

if(parenthesisStack[parenthesisStack.length-1] === '['){
parenthesisStack.pop();
} else {
parenthesisStack.push(']');
}


is repeated for each closing option. This violates the Don't Repeat Yourself principle strongly. Encapsulating it into a function would be a great idea.

The same goes to the opening values, they are always handled the same way, why not check if the input is one of them and give that the same treatment. You would only simplify code and ease future maintenance this way. Be aware that I am not really a fan of switches anywhere, except for enumerated types. But my advice is unbiased, as far as I feel it. You'd be better without a switch here, in my opinion.

## Minor

You have a // remove comment. It is quite pointless but it is too small to be an issue.

Lastly, it seems that the inline comment before the loop is misaligned. It would be nice to fix that.

## Algorithm-wise

Your algorithm only fails on the end. So if I start with } and follow it by () repeated a million times it will take a reasonable amount of time to terminate even though the string cannot possibly be valid.

• Thanks for the feedback. I have fixed it here: jsfiddle.net/rdesai/zq1Lnhkj/4 Apr 4 '16 at 3:12
• @RahulDesai Nothing to fix. Just space for improvement. It is better already. Do you see how it could be made shorter? With so many people behind crappy mobile connections, having smaller JS files is great (this is just another reason for avoiding redundant code, readability is still the main one). Apr 4 '16 at 3:44