Skip to content

Conversation

@nittoco
Copy link
Owner

@nittoco nittoco commented Aug 4, 2024

https://leetcode.com/problems/binary-tree-level-order-traversal/description/ Given the root of a binary tree, return the level order traversal of its nodes' values. (i.e., from left to right, level by level).

https://leetcode.com/problems/binary-tree-level-order-traversal/description/
Given the root of a binary tree, return the level order traversal of its nodes' values. (i.e., from left to right, level by level).
Copy link

@fhiyo fhiyo left a comment

Choose a reason for hiding this comment

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

良いと思います!

result = []
while nodes:
result.append([])
for _ in range(len(nodes)):
Copy link

Choose a reason for hiding this comment

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

range(len(nodes)) でループを回している中に nodes.popleft()node.append() のように nodes に変更を与えるような処理があるのがわかりにくいと感じました。このケースでは L.127 に到達した時点で len(nodes) が評価されるので、その後ループ内で nodes の長さを変更させてもループの回数は変わらないと思います。(自分で簡単なコードで試してみました)

ただ for node in nodes のようなコードの場合は、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.

これは確かにで、len(nodes)の評価はfor文回す前の最初にされるのは、自分も試してへ〜となる感じで、なんとなくで書いてしまってました。
これがPythonプログラマだったらすぐわかるとかだったらいいですが、そうでもないっぽいので別変数で用意した方がいいかもですね。

return result
```

- currentとnextの両方を持つことはせず、配列の要素数でlevelを管理
Copy link

Choose a reason for hiding this comment

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

シンプルに level を持たせるほうが可読性と保守性が高いかと思いました。コード全体を読めば一番最後のリストに要素を追加すればよいことは自明かもですが、もう少しコードの規模が大きくなると急に理解が難しくなりかつバグが発生しやすくなる気がします。result[level].append(node.val) の書き方だとこのコード単体でも意味がわかりやすいです。

Copy link
Owner Author

Choose a reason for hiding this comment

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

ありがとうございます。
TORUS0818/leetcode#28 (comment)
こういう意見もあるんですよね…何が理解のボトルネックになるのかというのが人によって違いそうで難しいです。
ただ、上の例だとlevel_ordered_valuesという変数名でlevel順というのがすぐわかるのに対して今回はすぐはわからないので、そういう違いもあるのかもしれません。

```python

from collections import deque

Choose a reason for hiding this comment

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

細かいですが、こちらは2行空けて、上の空行は不要な気がします。
https://peps.python.org/pep-0008/#blank-lines

class Solution:
def levelOrder(self, root: Optional[TreeNode]) -> List[List[int]]:
nodes_and_depth = deque()
nodes_and_depth.append((root, 0))

Choose a reason for hiding this comment

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

 nodes_and_depth = deque([(root, 0)])

でも良いですね。

return result
```

- 探索と同時に突っ込むのを考えて書いてみる
Copy link

Choose a reason for hiding this comment

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

同時に突っ込む方法は、上の形をもう少し形式的に変形することで書けませんか。

上のブロックで、nodes_val_ordered に append していて、下のブロックで nodes_val_ordered を for で回しているのだから、中身をappend のところに書けば理屈上動きそうです。

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.

6 participants