Skip to content

Conversation

@nittoco
Copy link
Owner

@nittoco nittoco commented Nov 27, 2024

問題文: https://leetcode.com/problems/minimum-depth-of-binary-tree/description/
Given a binary tree, find its minimum depth.

The minimum depth is the number of nodes along the shortest path from the root node down to the nearest leaf node.

Note: A leaf is a node with no children.

Comment on lines +27 to +33
if not node.left and not node.right:
return 1
if not node.right:
return self.minDepth(node.left) + 1
if not node.left:
return self.minDepth(node.right) + 1
return min(self.minDepth(node.left), self.minDepth(node.right)) + 1
Copy link

Choose a reason for hiding this comment

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

これ、まとまりますかね?

Copy link

Choose a reason for hiding this comment

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

あ、すみません、勘違いです。min ですね。

https://github.com/hroc135/leetcode/pull/21/files

- BFS、よく考えたらdepthはタプルに入れなくてもdepth+=1でよかった
- depth += 1をループの最初に書くか、最後に書くか、迷う
Copy link

Choose a reason for hiding this comment

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

私は最後派ですが、どちらでもいいのではないでしょうか。

Copy link
Owner Author

Choose a reason for hiding this comment

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

@oda さんはちなみにどういう理由で最後派ですか?
(結果としてどちらを採用するかはともかく、判断基準を持っておきたいなと思い…)

Copy link

Choose a reason for hiding this comment

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

C++ だったら気持ちとして、

for (int depth = 1; !current_level_nodes.empty(); ++depth) {
    ...
    current_level_nodes.swap(next_level_nodes);
}

じゃないですかね。

Copy link
Owner Author

Choose a reason for hiding this comment

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

なるほど、C++だとそういう書き方ができるんですね。
ありがとうございます。

Copy link

Choose a reason for hiding this comment

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

要するに、depth はループのテーマのようなもので、++depth というのがループを次の行に行くための処理であると見て、ループの中では 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.

そうですね、テーマが最初にいきなり変化するのは確かにちょっと追いにくいかもですね(ループを引き継ぎだと考えると)
ありがとうございます


```python
class Solution:
def is_leaf(self, node: [TreeNode]) -> bool:

Choose a reason for hiding this comment

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

node: [TreeNode]これはnode: Optional[TreeNode]の間違えでしょうか?

(その場合だと、nodeにNoneが来る想定になるので、TreeNodeの方がいいですね)

Copy link
Owner Author

Choose a reason for hiding this comment

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

型注釈の書き方がよくわかっておらず変な書き方をしてしまいました🙏
単なるTreeNodeのつもりできた

def minDepth(self, root: Optional[TreeNode]) -> int:
if not root:
return 0
current_level = [(root, 1)]

Choose a reason for hiding this comment

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

level(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.

なるほど、levelで終わってるとdepthしか入ってないように確かに見えるかもですね。
ありがとうございます。

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.

良いと思います。

continue
if self.is_leaf(node):
return depth
next_level_nodes.append(node.left)

Choose a reason for hiding this comment

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

自分はnode.leftがNoneかどうかはここでチェックしたいです。まあ好みかもしれないです。

current_level_nodes = next_level_nodes
```

- infの代わりに、まだ見てないかを別変数で定義

Choose a reason for hiding this comment

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

宗派にもよりそうですが、Pythonだとやり過ぎじゃないですかねー?C, C++歴が長い人は別変数にする方を好みそうな気もします。知らないですが。

if not root:
return 0
node_with_depth = [(root, 1)]
reached_leaf_so_far = False

Choose a reason for hiding this comment

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

なんか変数名が分かりづらいです。is_first, first_nodeあたりはどうですかね?

Choose a reason for hiding this comment

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

nittocoさん、変数名にso_farつけがちですが、自分はそんないい命名じゃないなーという気がしています。「これまでに」くらいの意味だと思っていて、具体性が低いのとあえて明示的に加えて嬉しい場面が少ない気がします。

たとえば、is_visited_so_farという変数名で「これまでに訪れたかどうかを」表そうとしても、is_visitedだけでもこのニュアンスは含まれていると思うんですよね。

Copy link
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 +197 to +201
if self.is_leaf(node):
if reached_leaf_so_far and depth >= result:
continue
result = depth
reached_leaf_so_far = True

Choose a reason for hiding this comment

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

if self.is_leaf(node) and (result is None or depth < result):
    result = 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.

それも考えたんですけど、Noneと整数を同じ変数にするのがどうかなと思ったんですよね(考えすぎですかね)

Choose a reason for hiding this comment

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

192行目でresult = Noneとなっていますが…

Copy link
Owner Author

@nittoco nittoco Nov 29, 2024

Choose a reason for hiding this comment

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

すみません、書いた時のことを思い出すとそういう判断基準ではなかったですね。
is Noneとただ書くより、別変数で定義した方が意図がわかりやすいと書いたときは思ってた気がします。

return 0
stack = [(root, 1)]
result = inf
while stack:

Choose a reason for hiding this comment

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

この解法だけstackといった命名を用いているのが気になりました。他に合わせてdepth_minやnode_and_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.

ありがとうございます そうですね、ちょっと不親切だったかもしれません

next_level_nodes.append(node.left)
if node.right:
next_level_nodes.append(node.right)
current_level_nodes = next_level_nodes

Choose a reason for hiding this comment

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

current_level_nodesとnext_level_nodeは一つに纏めてもいいなかと思いました。

Copy link
Owner Author

Choose a reason for hiding this comment

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

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.

7 participants