1
\$\begingroup\$

The task

I asked similar questions here and here.

Given a string of round, curly, and square open and closing brackets, return whether the brackets are balanced (well-formed).

For example, given the string "([])", you should return true.

Given the string "([)]" or "((()", you should return false.

My solution

...is inspired by @200_success' answer. I only made it more concise.

const isBracketBalanced = brc => {
  const braces = {
    "(": ")",
    "[": "]",
    "{": "}",
  }, expectCloseStack = [];

  const bracketIsBalanced = b => braces[b] ?
           expectCloseStack.push(braces[b]) :
           expectCloseStack.pop() === b;

  return [...brc]
    .every(bracketIsBalanced) &&
    !expectCloseStack.length;
};

console.log(isBracketBalanced("([])[]({})"));
\$\endgroup\$
  • \$\begingroup\$ I'm not going to try again... I've been told off by 200_success that mine was a bad solution and only to react to yours, not create my own. After a while he removed all his comments (second thoughts?). I got downvoted and someone wants my post deleted. Nice. But just wondering: Your solution looks quite optimal now, what do you expect from reviewers? To optimize it even further? The only thing I can think of is that it would be nice if you added an explanation of your code. It is quite difficult to disentangle. What's the algorithm, and why it is the best solution you've found so far? \$\endgroup\$ – KIKO Software Apr 13 at 18:16
  • \$\begingroup\$ Of course, I want others to suggest me a better solution or point what I could improve..... :) I usually don't comment my code because try to make my code as declarative as possible. If not, then I'd rather change my code than add a comment. @KIKOSoftware \$\endgroup\$ – thadeuszlay Apr 13 at 18:20
  • \$\begingroup\$ To me it doesn't look very declarative. Just remember, you've been working on this for days, it's obvious to you what it does. The minimum you could do is explain the algorithm used, just to help people understand your approach. \$\endgroup\$ – KIKO Software Apr 13 at 18:24
  • \$\begingroup\$ @KIKOSoftware according to you, what’s the most difficult part for you to understand? \$\endgroup\$ – thadeuszlay Apr 13 at 18:25
  • \$\begingroup\$ Yeah, I'm not going there. You know perfectly well that this is quite terse code, with lots of very specific syntactical elements. If you can't take a hint then forget it. \$\endgroup\$ – KIKO Software Apr 13 at 18:28
1
\$\begingroup\$

There are some issues with the code:

  • isBracketBalanced - Should be plural. It checks more than one bracket.
  • bracketIsBalanced - Is basically the same name as the parent function, except it does more than just check if the bracket is balanced. It also manipulates the stack.
  • braces - Didn't we just call them brackets?
  • expectCloseStack declaration - This is perhaps just my personal preference, but to me, writing the declaration after a comma like that makes me look twice to see if it is part of the object that was declared above it.

All in all, I find this solution a bit harder to read than the original. Also, it's actually not more concise.

\$\endgroup\$
  • \$\begingroup\$ What would be a good name? areBracketsBalanced? Why would you prefer switch-case over the "map"? \$\endgroup\$ – thadeuszlay Apr 13 at 20:17
  • \$\begingroup\$ Something like that. I don't prefer the switch statement, I just didn't like the extracted function. I think it would be better if you inlined it actually. It's hard to name it well \$\endgroup\$ – Magnus Jeffs Tovslid Apr 13 at 20:32
  • \$\begingroup\$ Hmm...probably a matter of taste. I prefer to extract function. In this case it makes it more readable (but I'm biased :P): every(bracketIsBalanced) is very descriptive for me and you can only do it in language where functions are first class citizens. I just think one has to take advantage of that. But that's just my opinion. \$\endgroup\$ – thadeuszlay Apr 13 at 20:35
  • \$\begingroup\$ True, it would be more readable if the function didn't have side effects. But since it does, it's harder to read because the name implies no side effects. Granted, the function is just a couple lines above, so it could be wourse \$\endgroup\$ – Magnus Jeffs Tovslid Apr 13 at 20:51
  • \$\begingroup\$ I get your point about the "side effects". \$\endgroup\$ – thadeuszlay Apr 13 at 20:52

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.