-
Notifications
You must be signed in to change notification settings - Fork 0
98. Validate Binary Search Tree #30
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
| # self.right = right | ||
| class Solution: | ||
| def isValidBST(self, root: Optional[TreeNode]) -> bool: | ||
| node_valid_range_pairs = [(root, -math.inf, math.inf)] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hoge_pairsだとpairの集合かな?と思ってしまいますが、これ変数名難しいですね
単純に stackにして、要素の性質に関してはコメントで書くくらいがちょうどいいでしょうか?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ありがとうございます。
確かに無理やり変数名で説明し尽くそうと考えなくても良いかもしれませんね。
これも選択肢として検討できるようになろうと思います。
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
node_and_ranges という変数名を考えましたが、微妙かもしれません。
| class Solution: | ||
| def isValidBST(self, root: Optional[TreeNode]) -> bool: | ||
| prev_val = -math.inf | ||
| is_bst = True |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is_validate_bst_helperがboolを返せるようにしてis_bstを消す方法もいけそうです
def isValidBST(self, root: Optional[TreeNode]) -> bool:
prev_val = -math.inf
def is_valid_bst_helper(node) -> bool:
if not node:
return True
nonlocal prev_val
if not is_valid_bst_helper(node.left):
return False
if prev_val >= node.val:
return False
prev_val = node.val
return is_valid_bst_helper(node.right)
return is_valid_bst_helper(root)There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
実装例もありがとうございます!
|
コメント以外良さそうです! |
| class Solution: | ||
| def isValidBST(self, root: Optional[TreeNode]) -> bool: | ||
| def is_valid_bst_helper( | ||
| root: Optional[TreeNode], lower_val: float, upper_val: float |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ノードの値の型が int であるにもかかわらず、下限と上限の変数の型に float を指定している点に違和感を感じました。 int に統一するとよいと思います。
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
math.infを受け入れられるようfloat型にしたのですが、sys.maxsizeを使えば素直にintにできますね。
| return is_valid_bst_helper(root.left, lower_val, root.val) \ | ||
| and is_valid_bst_helper(root.right, root.val, upper_val) | ||
|
|
||
| return is_valid_bst_helper(root, -math.inf, math.inf) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
一番小さい数と一番大きい数を表現するために math.inf を使っているのだと思うのですが、値の意味に関する説明がなく、ある種のマジックナンバーになってしまっているように思います。定数として MIN_VALUE と MAX_VALUE を定義し、それを使用するのはいかがでしょうか?
また、値として -sys.maxsize と sys.maxsize を使っている方もいらっしゃいます。これらを検討してみてはいかがでしょうか?
https://docs.python.org/ja/3/library/sys.html#sys.maxsize
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
math.infがマジックナンバーになる、という意識がありませんでした。
確かに一度意図がわかるよう変数にした方が良さそうです。
sys.maxsizeに関しては以前議論があったのは覚えているのですが、深く検討しませんでした。今後選択肢として考慮するようにします。
| # self.right = right | ||
| class Solution: | ||
| def isValidBST(self, root: Optional[TreeNode]) -> bool: | ||
| node_valid_range_pairs = [(root, -math.inf, math.inf)] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
node_and_ranges という変数名を考えましたが、微妙かもしれません。
| class Solution: | ||
| def isValidBST(self, root: Optional[TreeNode]) -> bool: | ||
| def is_valid_bst_helper( | ||
| root: Optional[TreeNode], lower_val: float, upper_val: float |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
この関数を見て、αβ法を思い出しました。ソフトウェアエンジニアの常識には含まれていないと思います。
https://ja.wikipedia.org/wiki/%E3%82%A2%E3%83%AB%E3%83%95%E3%82%A1%E3%83%BB%E3%83%99%E3%83%BC%E3%82%BF%E6%B3%95
kazukiii
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
すみません、遅くなりました!
| if root.left: | ||
| if not (lower_val < root.left.val < root.val): | ||
| return False | ||
| if root.right: | ||
| if not (root.val < root.right.val < upper_val): | ||
| return False |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
上流の関数から渡されたノード自体についてチェックを行うとよりシンプルかなと思いました。
if not lower_val < root.val < upper_val:
return FalseThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ありがとうございます!
確かにまとめて書けますね。
| 思考ログ: | ||
| - 二分探索木といえば、in-orderの実装を失念していた | ||
|
|
||
| in-order(ループ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
in-orderのループでの書き方勉強になりました。
https://leetcode.com/problems/validate-binary-search-tree/description/