5
\$\begingroup\$

The following code is a working solution for the balanced parentheses problem.

I'm really just wondering about coding style here when it comes to separating out related if-statements.

When there are two related if-statements, and where the second is run if the first is run, should I merge all checks into one complete if-statement, as in,

if char in open_close_parens and stack and stack[-1] == open_close_parens[char]:

or separate out the two if-statements as I've done here, with the check for an empty stack (list) then being more prominent in the second follow-up if-statement?

 def is_valid(stri: str) -> bool:
    stack = []
    open_close_parens = {
        ')':'(', 
        ']':'[', 
        '}':'{'
    }

    for char in stri:
            if char in open_close_parens:
                # if stack is not empty and there is a match, pop it
                if stack and stack[-1] == open_close_parens[char]:
                    stack.pop()
                else:
                    return False
            else:
                stack.append(char) 

    return not stack

print(is_valid(']'))
print(is_valid('()'))
\$\endgroup\$
1
  • 1
    \$\begingroup\$ Is "using nested if statements in loop" part of the code requirements, or just an artefact of how you solved the problem? If the latter, I'd recommend removing from the title. \$\endgroup\$ Jan 17 at 20:14

2 Answers 2

10
\$\begingroup\$

I would not combine those terms, but there are a few minor improvements.

  • Naming: Naming things is hard, but you have some improvements. You should try to use accurate terminology when writing code. You are not checking for parenthesis. You are checking for brackets, where parenthesis () is a special case of brackets.

    Similarly stri is a bad name. Something like brackets is better and because of the typing hint we know it is a string.

    pen_close_parens can be named bracket_pairs and should be a global constant. So BRACKET_PAIRS.

  • Early exits and continue: A way to clean up the logic is early exits and continue.

  • Tests are nicely placed inside of docstrings.

Code:

BRACKET_PAIRS = {")": "(", "]": "[", "}": "{"}


def balanced_brackets(brackets: str) -> bool:
    """Returns true if the number of brackets are balanced

    >>> balanced_brackets("]")
    False
    >>> balanced_brackets("()")
    True
    """
    stack = []

    for bracket in brackets:
        if bracket not in BRACKET_PAIRS:
            stack.append(bracket)
            continue

        # if stack is not empty and there is a match, pop it
        if not (stack and stack[-1] == BRACKET_PAIRS[bracket]):
            return False

        stack.pop()

    return not stack


if __name__ == "__main__":
    import doctest

    doctest.testmod()
\$\endgroup\$
6
  • \$\begingroup\$ @cheznead If you want to expand upon this you can check out this question: codereview.stackexchange.com/questions/270980/…. You can try the problem before looking at my solution =) The relevant part is check_bracket_balance. \$\endgroup\$ Jan 16 at 20:34
  • 1
    \$\begingroup\$ Learning something every day. In my mother language, brackets are a special case of parenthesis. Didn't know it is other way round in English. Thank you. \$\endgroup\$
    – Jakub
    Jan 16 at 20:43
  • 1
    \$\begingroup\$ @DavidZ Shrugs Just because one site uses a certain terminology does not make it right. I'd trust wikipedia more than leetcode when it comes to correct language en.wikipedia.org/wiki/Bracket. Now, In English and programming both the terms parenthesis and brackets are often used when talking about single instances. "You need to add some curly parenthesis here" (Note that just because we use it informally does not make it correct.) However, when we talk about a group of them, it is more correct to use bracket. Also square bracket is shortened to just bracket singularly. \$\endgroup\$ Jan 17 at 6:29
  • 1
    \$\begingroup\$ I would use DeMorgan on that second condition: if not stack or stack[-1] != BRACKET_PAIRS[bracket]:, maybe that's just a personal preference... \$\endgroup\$
    – Jasmijn
    Jan 17 at 12:04
  • 1
    \$\begingroup\$ @Jasmijn agree. The reason why I left it is that I really wanted to rewrite it as stack[-1] == BRACKET_PAIRS[bracket] is cryptc. Better would have been to have defined brackets_match = stack[-1] == BRACKET_PAIRS[bracket] and then done if not brackets_match. But I would then probably also rename stack to open_brackets (as in my linked codereview), but there are only so much time in a day to be pedantic... \$\endgroup\$ Jan 17 at 12:15
4
\$\begingroup\$

Remember doing this a while back. In my opinion, it is all good. It will work for this problem. The code is nice and easy to read. For small applications, I prefer code as you did with separated if statements, as it is easier to read and understand. Others may not agree, think that it's going to be down to personal preference. Really good points: type hints ;)

Don't know if you noticed but indentation is a bit off in your code, but that probably happened while pasting the code.

\$\endgroup\$

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.