Skip to content

Create merge_two_binary_trees.md#14

Open
tshimosake wants to merge 2 commits intomasterfrom
tshimosake-patch-12
Open

Create merge_two_binary_trees.md#14
tshimosake wants to merge 2 commits intomasterfrom
tshimosake-patch-12

Conversation

@tshimosake
Copy link
Owner

root.right = merge_two_nodes(node1.right, node2.right)
return root
else:
return node1 or node2

Choose a reason for hiding this comment

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

新しく作ったノードに、入力で受け取ったノードをつなぐことになる場合もあるかなと思いました。別の想定として、新しく作ったノードだけで構成された木を返す、ということを考えて書いてみるというのもよさそうと思いました。

もしdiscord 内やコメント集を調べてないようでしたら調べてみると、stack を使った書き方など違った書き方など見つかると思うのでお勧めです。

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
Owner Author

Choose a reason for hiding this comment

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

以下のように deepcopy を使えばいいかなと思いました。

else:    
    return copy.deepcopy(node1 or node2)

@shintaro1993
Copy link

もしファイル形式が.md以外の場合はこれにするとシンタックスハイライトが効くと思います。

@tshimosake
Copy link
Owner Author

すみません!mdにするのを忘れていました。後ほど修正しますm(_ _)m


2回目。dfs を merge_two_nodes へリネームした。

```py

Choose a reason for hiding this comment

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

ここpyではなくpythonにしておくと、レビュワーがレビューしやすくなると思います。

Copy link
Owner Author

Choose a reason for hiding this comment

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

pyでもシンタックスハイライトは効きますが、pythonであることがより明示的であるほうがよいとの意図でしょうか?

Copy link

Choose a reason for hiding this comment

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

拡張子が .md ファイルではなかったため、シンタックスハイライトが効いておらず、 py になっているからシンタックスハイライトが効いていないと勘違いしたのだと思いました。

Choose a reason for hiding this comment

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

pyでもシンタックスハイライト効くのですね
失礼しました

Copy link

Choose a reason for hiding this comment

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

```py
class Solution:
def mergeTrees(self, root1: Optional[TreeNode], root2: Optional[TreeNode]) -> Optional[TreeNode]:
def dfs(node1, node2):

Choose a reason for hiding this comment

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

dfsではなく、何をするものかが分かる命名にするとより分かりやすいコードになると思います。

root.left = dfs(node1.left, node2.left)
root.right = dfs(node1.right, node2.right)
return root
else:

Choose a reason for hiding this comment

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

何に対するelseかを遡るか、条件を覚えておく必要がある構造になっています。ので、elseの条件に当たるものを7行目あたりに持ってきて早期returmするのも良いと思います。その後に7行目以降の処理を書けば条件も消せますし。

Copy link
Owner Author

Choose a reason for hiding this comment

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

確かに、早期リターンすればそうでない場合のインデントを一つ浅くできますね!ありがとうございます!

@@ -0,0 +1,47 @@
1回目。答えを写した。
Copy link

Choose a reason for hiding this comment

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

LeetCode の答えにしないほうがいいと思います。時々変なことが書かれています。

完走している人から気に入った人を見繕っておくといいでしょう。

Copy link
Owner Author

Choose a reason for hiding this comment

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

かしこまりました、コメントありがとうございます!

@@ -0,0 +1,47 @@
1回目。答えを写した。

```py
Copy link

Choose a reason for hiding this comment

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

ファイルの拡張子を .md にすると、シンタックスハイライトが有効化され、読み手にとって読みやすくなると思います。

Copy link
Owner Author

Choose a reason for hiding this comment

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

すみません、忘れておりました、、、m(_ _)m

@tshimosake tshimosake changed the title Create merge_two_binary_trees Create merge_two_binary_trees.md May 26, 2025
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