Solved: 617. Merge Two Binary Trees

[Fuminiton]

617. Merge Two Binary Trees

[fuga-98]

Noneとの比較には is Noneを使うべきだと思います。
https://pep8-ja.readthedocs.io/ja/latest/#id41

また、 本当は if x is not None と書いているつもりで、 if x と書いている場合は注意してください - たとえば、デフォルトの値がNoneになる変数や引数に、何かしら別の値が設定されているかどうかをテストする場合です。この「別の値」は、ブール型のコンテクストでは False と評価される(コンテナのような)型かもしれませんよ！

[Fuminiton]

レビュー、リンクの共有までありがとうございます。

[oda]

一部のノードを共有すると、他の破壊的なメソッドとの相性で残念なことになるかもしれませんね。

[oda]

この場合、そのまま下に流せば、ノードが入力と出力で共有されませんね。

[Fuminiton]

deepcopyで返す or 下に流さないと、mergeTreesの返り値を操作したときに入力を書き換えてしまう可能性があるんですね。。見逃していました。

shining-ai/leetcode#23 (comment)

root2をルートとするツリーについては、引数で渡ってきたものをそのまま使用しているので、この書き方だとmergeTreesの返り値のTreeにroot2の一部が使われてしまいます。この関数自体では非破壊的かもしれないですが、mergeTreesの返り値のTreeの操作を行うとroot2のツリーを書き換えてしまう可能性があります。

[oda]

この場合、終端を表す番兵を使う手はありますよ。
TreeNode(0)
を node1, node2 にとりあえず入れておきます。

[Fuminiton]

ご指摘ありがとうございます。
あまり番兵を使ったことがなかったので避けていましたが、ここかなり簡潔になりますね。

            node1 = node1 if node1 else TreeNode(0)
            node2 = node2 if node2 else TreeNode(0)
            merged_node.val = node1.val + node2.val

[oda]

簡単になるのはここから下もです。

[Fuminiton]

以下の三項演算子の部分も単にnode1.rightで済むようになるのですね。

                    node1.right if node1 else None,
                    node2.right if node2 else None,

[oda]

if (node1 and node1.left) or (node2 and node2.left):

これもです。node1 node2 がある前提で書けます。

[Fuminiton]

おっしゃる通りですね。
番兵使った方が明らかに簡潔になりました。

ご指摘ありがとうございます。

SENTINEL = TreeNode(0)

class Solution:
    def mergeTrees(self, root1: Optional[TreeNode], root2: Optional[TreeNode]) -> Optional[TreeNode]:
        if root1 is None and root2 is None:
            return None

        merged = TreeNode()
        stack = [(root1, root2, merged)]
        while stack:
            node1, node2, merged_node = stack.pop()
            if node1 is None:
                node1 = SENTINEL
            if node2 is None:
                node2 = SENTINEL
            merged_node.val = node1.val + node2.val            
            if node1.left is not None or node2.left is not None:
                merged_node.left = TreeNode()
                stack.append((node1.left, node2.left, merged_node.left))
            if node1.right is not None or node2.right is not None:
                merged_node.right = TreeNode()
                stack.append((node1.right, node2.right, merged_node.right))
        return merged

[olsen-blue]

三項演算子が読み手の認知負荷を上げてしまう可能はなくはないです。
olsen-blue/Arai60#5 (comment)

[fuga-98]

Pythonの三項演算子読みにくいですね

[oda]

これ、私が感じるのは、なんで、わざわざ混ぜた後に分離するのか、です。

右手も左手も何も持っていない場合は、None を返してください。
右手に何も持っていない、または、左手に何も持っていない場合は、「右手に物を持っている場合は右手で持っているものを、そうでない場合は左手に持っているものを」返してください。

Python の三項演算子、目が左右に振られるんですよね。

[Fuminiton]

右手も左手も何も持っていない場合は、None を返してください。右手に何も持っていない、または、左手に何も持っていない場合は、「右手に物を持っている場合は右手で持っているものを、そうでない場合は左手に持っているものを」返してください。

確かに身近な言葉に直してみると、不自然な操作ですね
変な説明になっていないか考えながら実装しようと思います。

今回ですと、「右手に何も持っていない、または、左手に何も持っていない場合は、持っている方を返してください」（return root1 or root2）が自然かと思いました。

[oda]

私は、
右手が空なら左手で持っているものを返してください。
左手が空なら右手で持っているものを返してください。
でいいと思います。

[hroc135]

気になったところは全てコメント済みでした。いいと思います