Balanced parenthesis string checker

Question

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('Please enter a valid string.');
 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.

• Flambino: Related question

Answer 1

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.

Additionally,

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.

• Rahul Desai: Thanks for the feedback. I have fixed it here: jsfiddle.net/rdesai/zq1Lnhkj/4
• Bernardo Sulzbach: @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).