Skip to content

Conversation

@TORUS0818
Copy link
Owner

node = nodes.pop()
if from_right:
zigzag_level_ordered_values[-1].append(node.val)
append_node_if_exist(node.right)
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.

はい、どうかなと思ったんですが、やはり突っ込まれましたね。

Copy link

Choose a reason for hiding this comment

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

確かに重複多いですが、中身が2行だと関数にするか難しいところですよね。

Comment on lines +115 to +116
zigzag_level_ordered_values[level] \
= zigzag_level_ordered_values[level] + [node.val]
Copy link

Choose a reason for hiding this comment

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

イコールの前で改行する&イコールの位置が前の行の頭と同じなのあまり見ない気がしました。書くならこっち↓の方が自然かも?

zigzag_level_ordered_values[level] = \
    zigzag_level_ordered_values[level] + [node.val]

あとelse側と書き方を揃えたのかなと思いつつ、無駄にコピーが走るのでこちらは

zigzag_level_ordered_values[level].append(node.val)

でいい気がしました。

return []

zigzag_level_ordered_values = []
def zigzag_level_order_helper(nodes: list[Optional[TreeNode]], level: int) -> None:
Copy link

Choose a reason for hiding this comment

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

nodesの型、 list[TreeNode] じゃないですか?nodesの要素にNoneが入るケースは無い気がします

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 +201 to +202
num_same_level_nodes = len(nodes)
for _ in range(num_same_level_nodes):
Copy link

Choose a reason for hiding this comment

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

num_same_level_nodesは作る必要ない気がします。 for _ in range(len(nodes)) で良いと思いました。さらに、 for node in reversed(nodes) でpopしないか while nodes でpopするで良い気がしました。

Comment on lines +118 to +119
zigzag_level_ordered_values[level] \
= [node.val] + zigzag_level_ordered_values[level]
Copy link

Choose a reason for hiding this comment

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

zigzag_level_ordered_values[level].insert(0, node.val) の方がコピーを生成しない分効率が良いと思います。

Comment on lines +147 to +148
while level >= len(zigzag_level_ordered_values):
zigzag_level_ordered_values.append([])
Copy link

Choose a reason for hiding this comment

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

レベル順に見ていっているので zigzag_level_ordered_values.append([]) だけでも個人的にはパズル感はありませんが、どうでしょうか。

if from_right:
values.reverse()

zigzag_level_ordered_values.append(values)
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.

FBありがとうございます!

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