Conversation
98/sol1_dfs_recursion.py
Outdated
|
|
||
| class Solution: | ||
| def isValidBST(self, root: Optional[TreeNode]) -> bool: | ||
| def isValidBST_with_range(node, lower_bound, upper_bound): |
There was a problem hiding this comment.
LeetCodeで与えられているものに引数を加えたバージョン、という意図かと予想しますが、変数名は基本的に snake_case にした方が良いと思います。
https://peps.python.org/pep-0008/#function-and-variable-names
Function names should be lowercase, with words separated by underscores as necessary to improve readability.
There was a problem hiding this comment.
次回からはこのような場合でもsnake_caseにしようと思います。
98/sol1_dfs_recursion.py
Outdated
| def isValidBST_with_range(node, lower_bound, upper_bound): | ||
| if node is None: | ||
| return True | ||
| if node.val <= lower_bound or node.val >= upper_bound: |
There was a problem hiding this comment.
ただの感想ですが個人的には step 2 のように、 (nodeが範囲内) の否定、の方がコードと脳内での言語化の距離が近くてわかりやすいように思います。
There was a problem hiding this comment.
そうですね、自分もそう思ってsol2でロジックを変えました
| frontiers.append((node.left, lower_bound, node.val)) | ||
| if node.right is not None: | ||
| frontiers.append((node.right, node.val, upper_bound)) | ||
| return True |
There was a problem hiding this comment.
個人的な好みですが、whilte や for 文などの後には一つ空行を入れるようにしています。たまに目が滑って、while 文中だと勘違いしてしまうので 😓
There was a problem hiding this comment.
なるほど、勉強になります。他の方でもこのようにされている方は多そうですね。
https://peps.python.org/pep-0008/#blank-lines
にも関連しそうなことが書かれていますね。
98/sol2_dfs_stack.py
Outdated
| return True | ||
|
|
||
| frontiers = [(root, -float("inf"), float("inf"))] | ||
| while frontiers: |
There was a problem hiding this comment.
frontier を単数形にするか複数形にするかは議論があるようですね。私も過去指摘されたことですが、もしまだ目を通されていないなら、Discord内のレビューを見てみると良いかもしれません。
There was a problem hiding this comment.
単数形の方が良さそうです。次回以降こちらを採用します。
合わせてこのfrontierという名前に慣れておこうと思います。
There was a problem hiding this comment.
修正しました。修正前のコードではテストに通りませんでした(違うコードをpushしてしまったようです)。
98/sol1_dfs_recursion.py
Outdated
| return isValidBST_with_range( | ||
| node.left, lower_bound, min(upper_bound, node.val) | ||
| ) and isValidBST_with_range( | ||
| node.right, max(lower_bound, node.val), upper_bound | ||
| ) |
There was a problem hiding this comment.
min と max は取るまでもなく結果が分かっていそうです。
| return isValidBST_with_range( | |
| node.left, lower_bound, min(upper_bound, node.val) | |
| ) and isValidBST_with_range( | |
| node.right, max(lower_bound, node.val), upper_bound | |
| ) | |
| return isValidBST_with_range( | |
| node.left, lower_bound, node.val | |
| ) and isValidBST_with_range( | |
| node.right, node.val, upper_bound | |
| ) |
There was a problem hiding this comment.
上段でチェックしているので、その通りですね。修正しました。
98/sol2_dfs_stack.py
Outdated
| frontiers = [(root, -float("inf"), float("inf"))] | ||
| while frontiers: | ||
| node, lower_bound, upper_bound = frontiers.pop() | ||
| if not lower_bound < node.val < upper_bound: |
There was a problem hiding this comment.
lower_bound や upper_bound はinclusiveなのかexclusiveなのか分かりにくいので、_exclusive と変数名に明示するか、must_be_less_than のような直接的な変数名にしたいです。
There was a problem hiding this comment.
must_be_less_than、良い名前だと思ったので採用します
98/sol3_inorder.py
Outdated
| def push_it_and_left_children(node): | ||
| if node is None: | ||
| return | ||
| while node.left is not None: |
There was a problem hiding this comment.
末端を積みそこねてしまいそうです。
| while node.left is not None: | |
| while node is not None: |
98/sol3_inorder.py
Outdated
| if node.right is not None: | ||
| push_it_and_left_children(node.right) |
There was a problem hiding this comment.
push_it_and_left_children 内でNoneチェックをしているので、ここでは不要ですね。
| if node.right is not None: | |
| push_it_and_left_children(node.right) | |
| push_it_and_left_children(node.right) |
https://leetcode.com/problems/validate-binary-search-tree/submissions/1952536263/