Skip to content

104, Maximum Depth of Binary Tree #14

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Conversation

nittoco
Copy link
Owner

@nittoco nittoco commented May 30, 2024

URL: https://leetcode.com/problems/maximum-depth-of-binary-tree/description/
問題文: Given the root of a binary tree, return its maximum depth.

A binary tree's maximum depth is the number of nodes along the longest path from the root node down to the farthest leaf node.

URL: https://leetcode.com/problems/maximum-depth-of-binary-tree/description/
問題文: Given the root of a binary tree, return its maximum depth.

A binary tree's maximum depth is the number of nodes along the longest path from the root node down to the farthest leaf node.
Copy link

@hayashi-ay hayashi-ay left a comment

Choose a reason for hiding this comment

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

BFSはqueueを使って解いてみても良いですね。

- 最初に思いついた解法
- depthを中でも参照するために、returnでdepth, max_depthの両方を返してしまったが、そもそもclassなのでself.で再帰関数を定義すればよかった
- 最初はreturn depth_search(depth, root, max_depth)[1]だったが見にくい気がして直す
- なぜか実行速度も45ms→33msに

Choose a reason for hiding this comment

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

Pythonの実行速度は実行ごとに結構変わるので何回か実行して比較してみると良いと思います。

Copy link
Owner Author

Choose a reason for hiding this comment

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

本当ですね、33msだったのが今度は47msになったりします。
これくらいだと測定誤差ですね。ありがとうございます。


```python

class Solution:

Choose a reason for hiding this comment

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

だいぶ読みにくいです。表層的にはdepth, max_depthを取り回している部分をやめるとある程度改善されそうな気がしています。

Choose a reason for hiding this comment

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

このコードは一発で書けましたか?何回かfailしましたか?自分はこのコードを1回で書ける自信がないです。

Copy link
Owner Author

Choose a reason for hiding this comment

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

これは、
1,まずreturnをせずにうっかり書いてしまう
2,failした、よく考えたらreturnをしないとPythonでは変数はローカルなので保存されないんだった
という流れで書きました。

「depth, max_depthの値を保存するぞ」という意志のもと2で直したので、その変数をたくさん使ってしまい、上書き処理が多くなってしまいました。

自分の中ではdepthがそれぞれの関数のなかでそれぞれの深さを表しているつもりで、そんなに詰まらず書いてしまったのですが、上書き処理が多くなったせいで読む側にとってはわかりにくくなってしまったのかもしれません。あと、関数の処理の最初ではなく引数の時点で足すという選択肢がなかったです。大変参考になります。

Copy link
Owner Author

Choose a reason for hiding this comment

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

ちなみに、同じ変数名で値が頻繁に変わるコードは一般に読みにくいと思われるのでしょうか。
今回の場合、depthが「現在の深さ」という意識のもとで読んでるのに、それが頻繁に変わるから混乱する、という感じでしょうか。

Choose a reason for hiding this comment

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

値が変わること自体はそこまで問題でなくて変数の意味が変わるので読みにくいのかなと思いました。

同じdepthという変数名でもそれぞれ以下の意味あいがあるはずで、読む側としてはいまのdepthが表しているものを文脈に応じて考える必要があって結構大変です。

L12:1つ上のノードの深さ
L15:現在のノードの深さ
L17の左側:左側の子の深さ
L17の右側:現在のノードの深さ
L19の左側:右側の子の深さ
L19の右側:現在のノードの深さ
L21:現在のノードの深さ

Copy link

@liquo-rice liquo-rice May 31, 2024

Choose a reason for hiding this comment

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

nittocoさんのアルゴリズムを別のやり方で実装するとこんな感じになりますかね。

class Solution:
    def maxDepth(self, root: Optional[TreeNode]) -> int:
        depth = 0
        max_depth = 0
        def find_max_depth(node):
            nonlocal depth, max_depth
            if not node:
                return
            depth += 1
            max_depth = max(depth, max_depth)
            find_max_depth(node.left)
            find_max_depth(node.right)
            depth -= 1
        
        find_max_depth(root)
        return max_depth

Choose a reason for hiding this comment

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

そもそもreturn depth, max_depthのdepthは必要なさそうな気がします。

Copy link
Owner Author

@nittoco nittoco Jun 1, 2024

Choose a reason for hiding this comment

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

確かに最初と最後で+1, -1をする方が綺麗ですね
nonlocalは(https://realpython.com/python-pass-by-reference/)
だと非推奨でしたがこのくらいの規模のコードだといいのかも
Step4やる時に、お2人のコードを参照に実装してみます。ありがとうございます。(depthいらないのは後で気づきました)

Choose a reason for hiding this comment

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

nonlocalが非推奨ってどこに書いてますか?

Copy link
Owner Author

Choose a reason for hiding this comment

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

おお、nonlocalとglobalって違うんですね、今気づきました。ありがとうございます
https://docs.python.org/ja/3/reference/simple_stmts.html#global
上の記事だとglobalは非推奨と書いてありますね。(https://realpython.com/python-pass-by-reference/#replicating-pass-by-reference-with-python)
nonlocalは今見た感じ、入れ子になった関数内で外側の変数を参照して変えるということですかね。

if not current_node:
return depth, max_depth
max_depth = max(depth, max_depth)
depth, max_depth = depth_search(depth, current_node.left, max_depth)

Choose a reason for hiding this comment

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

Suggested change
depth, max_depth = depth_search(depth, current_node.left, max_depth)
left_depth, left_max_depth = depth_search(depth + 1, current_node.left, max_depth)

この2つである程度改善されないですかね?

  • 変数として受取る際に既存を上書きするのではなく、新しい変数にしてあげる。
  • 関数の引数に渡す際に足してあげる。

return depth, max_depth
max_depth = max(depth, max_depth)
depth, max_depth = depth_search(depth, current_node.left, max_depth)
depth -= 1

Choose a reason for hiding this comment

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

このdepthを1減らしている処理だけでも理解するのが結構しんどかったです。

引数のdepthは1つ上のノードの深さで、再帰関数の先頭で+1しているのでその時点で現在のノードの深さになっているが、直前のdepth_searchの呼び出しで1つ下のノードの深さが帰ってきているので現在のノードの深さに戻すために1を引く、処理を全部追ってここまでしないと-1をしている理由が分からないです。

if not root:
return 0
depth = 1
current_node = [root]

Choose a reason for hiding this comment

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

listなのでcurrent_nodesと複数形の方がよいです。currentがあることであまり意味は追加されていない気がするのでなくても良いかなとも思います。

Choose a reason for hiding this comment

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

下では複数形にしてますね。

max_depth = max(max_depth, depth)
if node.right:
node_depth.append((node.right, depth+1))
max_depth = max(max_depth, depth)

Choose a reason for hiding this comment

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

ifの前でmax_depthを求めているので不要な気がします。

Choose a reason for hiding this comment

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

下では消してますね。

```

- 再帰(これは簡単)
- なぜかmax_depth = max(self.maxDepth(root.left), self.maxDepth(root.right)) + 1と一気に書くよりだいぶ遅い(一気に書くのは27ms、下のは40ms)

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