Skip to content

102. Binary Tree Level Order Traversal#26

Open
tarinaihitori wants to merge 2 commits into
mainfrom
102-binary-tree-level-order-traversal
Open

102. Binary Tree Level Order Traversal#26
tarinaihitori wants to merge 2 commits into
mainfrom
102-binary-tree-level-order-traversal

Conversation

@tarinaihitori
Copy link
Copy Markdown
Owner

nodes_count = len(nodes_queue)
for _ in range(nodes_count):
node = nodes_queue.popleft()
same_level_nodes.append(node.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.

この変数に Node ではなくて val が入っているのはちょっと驚きがありました。

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.

たしかに、nodeにnode.valがあるのは読み手にびっくりさせてしまいますね。

queue_length = len(queue)
for _ in range(queue_length):
node = queue.popleft()
same_level_nodes.append(node.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.

ここnodeではなくnode.valなの違和感あります。
same_level_node_valsの方が適切かと。

nodes = [root]
while nodes:
current_level_nodes = []
next_level_nodes = []
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

ここ、nodes_by_level.append([])として、nodes_by_level[-1]にappendしていくこともできます。

@@ -0,0 +1,95 @@
1st
queue を用意し、同じレベルの node を queue にいれ、ノードの要素数だけループを回してリストに入れる。
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

Choose a reason for hiding this comment

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

悪くはないんですが、要するに一つの変数に、2種類のもの(現在のレベルと次のレベル)が入っていて、その境界が先頭から queue_length - _ の場所っていうことですよね。
これって書いている人はともかく、読んでいる人はコード全体読まないと分からなくないですか。
いまは、6行だからまだいいですが、ここが100行とかになると、すべての行が queue_length - _ と queue の内部の状態の関係の制約を守っていることを確認するのは手間です。
https://docs.google.com/document/d/11HV35ADPo9QxJOpJQ24FcZvtvioli770WWdZZDaLOfg/edit?tab=t.0#heading=h.ho7q4rvwsa1g


再帰的な方法。
動くコードはかけるが、頭で処理を追う場合BFS的な方がしっくりきますね。
node の数が 2000 までなので RecursionError は考慮しなくていい。
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

正常な入力の場合、RecursionErrorは送出されませんね。
ただ、例えば間違って葉ノードが全て根ノードを指すようになっていたとしたら、再帰で書いておくとRecursionErrorで異常な入力を検知することができますね。

Comment on lines +69 to +71
result = []
traverse_by_level(root, 0)
return result
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

好みをいうと、inner function(今回だとtraverse_by_level())内で、その引数にない、enclosing function(今回だとlevelOrder())の変数をいじるのはあまり好きではないです。
このサイズなら問題ないですが、大きくなった時、何がどこで変化したのか分かりにくいからです。
じぶんなら、resultも引数に入れるか、traverse_by_level()の返り値をresultになるように調整するかなと思います。

Comment on lines +79 to +81
nodes_by_level = []
if root is None:
return nodes_by_level
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:
            return []
        nodes_by_level = []

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