Skip to content

98 validate binary search tree#28

Open
tarinaihitori wants to merge 3 commits into
mainfrom
98-validate-binary-search-tree
Open

98 validate binary search tree#28
tarinaihitori wants to merge 3 commits into
mainfrom
98-validate-binary-search-tree

Conversation

@tarinaihitori
Copy link
Copy Markdown
Owner

node_stack.append(node)
node = node.left
node = node_stack.pop()
if node.val <= prev_val:
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

好みの範囲だと思いますが、
if not prev_value < node.val:のが理解しやすい気がします。

<https://github.com/Fuminiton/LeetCode/pull/28/files>

in-order traversalを用いて書いてみるが、書けなかった。
コードを読んでなぜそれで動くのかはわかるが、
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

自分は簡単な深さ3くらいのBSTを用意して、再帰の動きを理解した上でループに書き直すにはどうするか考えることで何となく書けるようになりました。

具体的には、
各頂点の横にvisited(その頂点に関する処理を行う), left(現在の頂点の左をの子を見にいく), rightとメモ書きしておいて、pre-order,in-order,post-orderの走査はそれぞれvisited,left.rightをどういう順番でチェックすることになるかを追っていきました。

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

たしかこのあたりから飛べるあたりで、どう書いたら少しは読みやすいか話していた気がします。
https://docs.google.com/document/d/11HV35ADPo9QxJOpJQ24FcZvtvioli770WWdZZDaLOfg/edit?tab=t.0#heading=h.wsy49qwpwhfr

```python
class Solution:
def isValidBST(self, root: Optional[TreeNode]) -> bool:
def is_valid_bst_helper(node, low=-(sys.maxsize), high=sys.maxsize):
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

64bitの符号付き整数の最小値は-2**63となるので、
-sys.maxsize - 1としてあげた方が正確な気がします。

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

-math.infのまま書いてしまっていましたが、
確かに、負の数の場合は-1してあげるほうがより正確そうです。

return is_valid_bst_helper(node.left, low, node.val) \
and is_valid_bst_helper(node.right, node.val, high)
return is_valid_bst_helper(root)
```
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

いいと思います。
範囲に収まるか確認していく方式が自分も好みでした。

in-order順にvalを返すgeneratorを使う実装も、
シンプルで直感的なのでやってみてもいいと思います。

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

generatorあまり知らなかったので、典型コメント集から飛んでみましたが
すごく直感的だったのでstep4以降でgeneratorを使った方法も実装しようと思います。

```python
class Solution:
def isValidBST(self, root: Optional[TreeNode]) -> bool:
def _is_valid_bst(node, low =-(math.inf), high = math.inf):
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

関数内関数の定義は1行空けたほうが見やすいと個人的に思いました。PEP8 に関数内関数に対する直接の言及はないようですが、ロジックの単位で改行を使うことは推奨されていそうです。

https://peps.python.org/pep-0008/#blank-lines

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ありがとうございます。
ここどうしようか自分でも迷っていたのですが、今後は1行あけるようにします。

if root is None:
return True

nodes_with_bounds = deque([(root, -(math.inf), math.inf)])
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

-(math.inf)() は不要そうです。() がたくさん出てくると区切りがわかりづらいので個人的にはなくてもよいものは削除したい派です。

prev_val = -(sys.maxsize)

while node_stack or node is not None:
while node:
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

細かいですが、同じチェックをするときに if root is None という表現と while node という表現が混在しているのがちょっと気になりました。統一感があるとよさそうです。

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

たしかにおっしゃる通りですね。

Comment on lines +94 to +95
val = node.val
if not (low < val < high):
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

val は汎用的な言葉なので node.val のまま扱うほうが可読性が高そうです。(置き換えによる嬉しさは書き手側の書く量が少し減ることかなと推測しましたが、読み手側目線だととくに嬉しさがなさそうです)
また low, high も上記で使っていたような lower_bound, upper_bound とするほうがよりわかりやすい気がします。

```python
class Solution:
def isValidBST(self, root: Optional[TreeNode]) -> bool:
def _is_valid_bst(node, low =-(math.inf), high = math.inf):
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

値が整数なので、sys.maxsizeの方が好みですが、デカい異物の初期値であればなんでもいいので math.inf でもいいと思います。

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

あ、下で検討されてますね。

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants