Skip to content

98. Validate Binary Search Tree#28

Open
TakayaShirai wants to merge 1 commit intomainfrom
98_validate_binary_search_tree
Open

98. Validate Binary Search Tree#28
TakayaShirai wants to merge 1 commit intomainfrom
98_validate_binary_search_tree

Conversation

@TakayaShirai
Copy link
Copy Markdown
Owner

@TakayaShirai TakayaShirai self-assigned this Feb 20, 2026
Copy link
Copy Markdown

@huyfififi huyfififi left a comment

Choose a reason for hiding this comment

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

全てのコードがわかりやすかったです。Dartのシンタックスを調べる良い機会になりました!

Comment on lines +63 to +86
class Solution {
bool isValidBST(TreeNode? root) {
num previousValue = double.negativeInfinity;

bool isValidBSTInorderTraverse(TreeNode? node) {
if (node == null) {
return true;
}

if (!isValidBSTInorderTraverse(node.left)) {
return false;
}

if (node.val <= previousValue) {
return false;
}
previousValue = node.val;

return isValidBSTInorderTraverse(node.right);
}

return isValidBSTInorderTraverse(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.

以下の2点がちょっとトリッキーに感じました。

  • ヘルパー関数の外側にある previousValue を使って「前」の値を保存しておく
  • 左右の子を辿る途中で、昇順かどうかのチェックをしている

個人的には、inorderで値を取り出すところと、昇順かチェックするところは分離する方が読みやすいと思いました。以下はGeminiに生成させたものなのでdart的な良し悪しはわかりませんがご参考まで。

class Solution {
  bool isValidBST(TreeNode? root) {
    if (root == null) return true;

    Iterable<int> traverseInorder(TreeNode? node) sync* {
      if (node == null) return;

      yield* traverseInorder(node.left);
      yield node.val;
      yield* traverseInorder(node.right);
    }

    num previousValue = double.negativeInfinity;
   
    for (final currentValue in traverseInorder(root)) {
      if (currentValue <= previousValue) {
        return false;
      }
      previousValue = currentValue;
    }

    return true;
  }
}

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 +125 to +128
final isLarger = min < node.val;
final isSmaller = node.val < max;

if (!isLarger || !isSmaller) {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

変数に置きすぎで、数直線上に並べるくらいでも十分伝わりそうです。

Suggested change
final isLarger = min < node.val;
final isSmaller = node.val < max;
if (!isLarger || !isSmaller) {
if (!(min < node.val && node.val < max)) {

Comment on lines +147 to +149
var nodesWithLimits = <(TreeNode, num, num)>[
(root, double.negativeInfinity, double.infinity),
];
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

(単に、個人の好みの表明です)

初期値の設定やpopのときに何が入っているか分かるので、このスタック(場合によってはキュー)には stack 以上の情報を持たせてもさほど意味は無いんじゃないかな(むしろ複雑な名前がノイジーになるのでは)、と最近感じるようになりました。

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.

レビューありがとうございます。

練習会を進めていく上で考えが変わるかもしれませんが、名前に情報は持たせておいた方がいいのではないかなと個人的には思います。例えば、一年後に、この関数を作成した背景を忘れた状況で、スタックやキューの中身から役割を類推するのが意外に難しそうと感じるのと、人によって変数の役割の理解が異なってしまったときに、勘違いが起こりミスにつながりそうと感じるためです。

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.

3 participants