20. Valid Parentheses - #6
Conversation
| def isValid(self, s: str) -> bool: | ||
| stack = [] | ||
| bracket_pairs = {"(": ")", "{": "}", "[": "]"} | ||
|
|
||
| for char in s: | ||
| if char in bracket_pairs: | ||
| stack.append(char) | ||
| continue | ||
| if stack: | ||
| if char != bracket_pairs[stack[-1]]: | ||
| return False | ||
| else: | ||
| stack.pop() | ||
| else: | ||
| return False | ||
| return not stack |
There was a problem hiding this comment.
個人的にはfor文内の条件分岐部分についてネストを浅くしたいなと思いました。
| def isValid(self, s: str) -> bool: | |
| stack = [] | |
| bracket_pairs = {"(": ")", "{": "}", "[": "]"} | |
| for char in s: | |
| if char in bracket_pairs: | |
| stack.append(char) | |
| continue | |
| if stack: | |
| if char != bracket_pairs[stack[-1]]: | |
| return False | |
| else: | |
| stack.pop() | |
| else: | |
| return False | |
| return not stack | |
| def isValid(self, s: str) -> bool: | |
| stack = [] | |
| bracket_pairs = {"(": ")", "{": "}", "[": "]"} | |
| for char in s: | |
| if char in bracket_pairs: | |
| stack.append(char) | |
| continue | |
| if not stack: | |
| return False | |
| expected_bracket = bracket_pairs[stack[-1]] | |
| if char != expected_bracket: | |
| return False | |
| stack.pop() | |
| return not stack |
There was a problem hiding this comment.
ありがとうございます！
else 節がなくなることで見通しがよくなった印象です！
Step 4 として追加しました！
https://github.com/kazizi55/coding-challenges/pull/6/files#diff-dee0cfbbf6f296838839ded17654fe00273d500efa9a5fdb004e516117cecc86R18-R35
| if char in bracket_pairs: | ||
| stack.append(char) | ||
| continue | ||
| if char != bracket_pairs[stack[-1]]: |
There was a problem hiding this comment.
個人的にbracket_pairs[stack[-1]]が少し認知負荷が高いと感じました。
変数の命名により明示的にすると緩和できると思いました。
| if char != bracket_pairs[stack[-1]]: | |
| expected_bracket = bracket_pairs[stack[-1]] | |
| if char != expected_bracket: |
There was a problem hiding this comment.
ありがとうございます。
確かに改めて見ると認知負荷が高いですね、、「stack の一番後ろにあるということは opening bracket を期待しているのだな」という推察をしてもらわないといけないですもんね。
以下で説明変数を追加しました！
https://github.com/kazizi55/coding-challenges/pull/6/files#diff-dee0cfbbf6f296838839ded17654fe00273d500efa9a5fdb004e516117cecc86R1-R16
| @@ -0,0 +1,45 @@ | |||
| class SolutionWithBracketPairsAndSentinel: | |||
| def isValid(self, s: str) -> bool: | |||
| stack = ["*"] | |||
There was a problem hiding this comment.
通常の list を stack として扱うということを明示する意図でした。
https://github.com/kazizi55/coding-challenges/pull/6/files#diff-2fbd326bf14883ff067365f9862ec32c829032750dd33fcf9caef79989b432d6R23
There was a problem hiding this comment.
中にどのような要素が含まれるかを想起できるような英単語、または英語句を付けたほうが、読み手にとってコードを理解するうえで有用な情報になると思います。
|
|
||
| ## Comments | ||
|
|
||
| ### Step 1 |
There was a problem hiding this comment.
Step1で何を考えて解いていたかもう少し書くとコメントがしやすいと思いました！
どういう思考・試行を経て回答に至っているかのほうが気になるので
There was a problem hiding this comment.
ありがとうございます！
確かに Step 1についても思考過程を詳しく書いた方が議論が活発になりそうですね。
次回から追加してみます。
| return False | ||
| return len(stack) == 1 | ||
|
|
||
| # bracket pairs を使って実装する version (expected_closing_bracket の命名を追加し、else を使わない) |
There was a problem hiding this comment.
番兵使わないほうが、この場合は素直かもしれませんね。あると少し説明がいる感じがします。その割に減るのが条件文ひとつなので。
底に "*" をいれているのが、特別な意味がありそうに見えているというのもあると思っています。
"" や None のほうが特別な意味がない感じがするので、読み手には少し楽です。
| @@ -0,0 +1,45 @@ | |||
| class SolutionWithBracketPairsAndSentinel: | |||
| def isValid(self, s: str) -> bool: | |||
| stack = ["*"] | |||
There was a problem hiding this comment.
中にどのような要素が含まれるかを想起できるような英単語、または英語句を付けたほうが、読み手にとってコードを理解するうえで有用な情報になると思います。
| class SolutionWithBracketPairsAndSentinel: | ||
| def isValid(self, s: str) -> bool: | ||
| stack = ["*"] | ||
| bracket_pairs = {"(": ")", "{": "}", "[": "]", "*": ""} |
There was a problem hiding this comment.
個人的には開きカッコと閉じカッコの対応であることを読み手に伝えるため、 open_to_close と名付けると思います。好みの問題かもしれません。
| def isValid(self, s: str) -> bool: | ||
| stack_list = list() | ||
|
|
||
| for char in s: |
There was a problem hiding this comment.
char という変数名は、 C/C++ で予約語となっているため、避けたほうが無難かもしれません。 c や ch といった変数名を自分はよく使います。
6 participants
https://leetcode.com/problems/valid-parentheses/description/
Next: https://leetcode.com/problems/reverse-linked-list/