Conversation
mamo3gr
reviewed
Jan 13, 2026
| - スタックは1つにする.スタックに(新しい木の現在ノード, roo1のノード, root2のノード)を入れる. | ||
| スタックに格納されている3つの要素は同じ位置のノードを表す. | ||
|
|
||
| 時間計算量: O(min(m, n)), 空間計算量: O(min(m, n)) |
There was a problem hiding this comment.
こうなる理由も書き留めておくと良いと思いました。m, n はそれぞれ木のノード数でしょうか。
どうにしろすべてのノードを見に行く必要があるので、minは取れなさそうな気がしました。
mamo3gr
reviewed
Jan 13, 2026
Comment on lines
+23
to
+26
| if node.left is None: | ||
| left = None | ||
| else: | ||
| left = node.left |
There was a problem hiding this comment.
個人的には新しい変数は一番上の（浅い）インデントで出てきたほうが驚きが小さいです。
Suggested change
| if node.left is None: | |
| left = None | |
| else: | |
| left = node.left | |
| left = None | |
| if node.left is not None: | |
| left = node.left |
Suggested change
| if node.left is None: | |
| left = None | |
| else: | |
| left = node.left | |
| left = None if node.left is None else node.left |
mamo3gr
reviewed
Jan 13, 2026
Comment on lines
+221
to
+233
| merged = TreeNode() | ||
| root1_left = None | ||
| root1_right = None | ||
| root2_left = None | ||
| root2_right = None | ||
| if root1 is not None: | ||
| merged.val += root1.val | ||
| root1_left = root1.left | ||
| root1_right = root1.right | ||
| if root2 is not None: | ||
| merged.val += root2.val | ||
| root2_left = root2.left | ||
| root2_right = root2.right |
There was a problem hiding this comment.
None で初期化される変数がいきなり4つも出てきて身構えてしまいました。それぞれ扱う場所の近くに置くのはどうでしょうか。
Suggested change
| merged = TreeNode() | |
| root1_left = None | |
| root1_right = None | |
| root2_left = None | |
| root2_right = None | |
| if root1 is not None: | |
| merged.val += root1.val | |
| root1_left = root1.left | |
| root1_right = root1.right | |
| if root2 is not None: | |
| merged.val += root2.val | |
| root2_left = root2.left | |
| root2_right = root2.right | |
| merged = TreeNode() | |
| root1_left = None | |
| root1_right = None | |
| if root1 is not None: | |
| merged.val += root1.val | |
| root1_left = root1.left | |
| root1_right = root1.right | |
| root2_left = None | |
| root2_right = None | |
| if root2 is not None: | |
| merged.val += root2.val | |
| root2_left = root2.left | |
| root2_right = root2.right |
あるいは「mergedに値を足し込む」のと「（あれば）子ノードを取り出す」という作業に分離するのも良いかもしれません。（同じ条件分岐が繰り返されるのがやや冗長かもしれませんが）
Suggested change
| merged = TreeNode() | |
| root1_left = None | |
| root1_right = None | |
| root2_left = None | |
| root2_right = None | |
| if root1 is not None: | |
| merged.val += root1.val | |
| root1_left = root1.left | |
| root1_right = root1.right | |
| if root2 is not None: | |
| merged.val += root2.val | |
| root2_left = root2.left | |
| root2_right = root2.right | |
| merged = TreeNode() | |
| if root1 is not None: | |
| merged.val += root1.val | |
| if root2 is not None: | |
| merged.val += root2.val | |
| root1_left = None | |
| root1_right = None | |
| if root1 is not None: | |
| root1_left, root1_right = root1.left, root1.right | |
| root2_left = None | |
| root2_right = None | |
| if root2 is not None: | |
| root2_left, root2_right = root2.left, root2.right | |
mamo3gr
reviewed
Jan 13, 2026
| if root2 is None: | ||
| return root1 | ||
|
|
||
| def get_merged_child(node, child1, child2, attr): # attr = "left" or "right" |
There was a problem hiding this comment.
typing.Literal を使うと型チェッカーでも検査できるようになります。
Suggested change
| def get_merged_child(node, child1, child2, attr): # attr = "left" or "right" | |
| def get_merged_child(node, child1, child2, attr: Literal["left", "right"]): |
mamo3gr
reviewed
Jan 13, 2026
| setattr(node, attr, merged_child) | ||
| return merged_child | ||
|
|
||
| merged_head = TreeNode(root1.val + root2.val) |
There was a problem hiding this comment.
「head という新しい単語が出てきたからには、rootとは違う何か新しい役割があるのだろうな？」と思いました。既出の root の方が「単なるマージ先」であることが伝わりやすそうです。
Suggested change
| merged_head = TreeNode(root1.val + root2.val) | |
| merged_root = TreeNode(root1.val + root2.val) |
05ryt31
reviewed
Jan 13, 2026
| if root1 is None and root2 is None: | ||
| return None | ||
|
|
||
| if root1 is None: |
There was a problem hiding this comment.
@Yuto729
本来レビューで質問するのは良くないと思うのですが、一点だけ確認させてください。
step1ではroot1, root2がNoneである場合はもう一方をそのままReturnしておられたと思うのですが、ここではわざわざ新しくnodeという変数を使って再度改めてmergeTreesまで呼んでいる理由ってなんですか？メモを読んだのですがいまいちここの意図が掴めなくて。。。
Comment on lines
+103
to
+104
| left1, right1 = node1.left, node1.right | ||
| left2, right2 = node2.left, node2.right |
There was a problem hiding this comment.
自分だったら、ここであえて変数化せずそのまま引数に代入しちゃうと思います
その方がget_merged_childの処理が追いやすいので。
| if root2 is None: | ||
| return root1 | ||
|
|
||
| def get_merged_child(node, child1, child2, attr): # attr = "left" or "right" |
There was a problem hiding this comment.
child1, child2 の代わりに、child, the_other_child とするのはいかがでしょう？
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
解く問題
Merge Two Binary Trees
次に解く問題
Convert Sorted Array To Binary Search Tree