617. Merge Two Binary Trees by Ryotaro25 · Pull Request #25 · Ryotaro25/leetcode_first60

Ryotaro25 · 2024-08-01T05:46:21Z

問題へのリンク
https://leetcode.com/problems/merge-two-binary-trees/description/

問題文(プレミアムの場合)

備考

次に解く問題の予告
Convert Sorted Array to Binary Search Tree

フォルダ構成
LeetCodeの問題ごとにフォルダを作成します。
フォルダ内は、step1.cpp、step2.cpp、step3.cpp、dfs.cpp、new_node.cppとmemo.mdとなります。

memo.md内に各ステップで感じたことを追記します。

nittoco · 2024-08-01T09:18:51Z

617.MergeTwoBinaryTrees/new_node.cpp

+
+    while (!search_nodes.empty()) {
+      const auto [current_node, first_node, second_node] = std::move(search_nodes.front());
+      search_nodes.pop();


queueに、nodeがNoneの時も突っ込んじゃって、pop()してから判定する方法もありますね(https://github.com/TORUS0818/leetcode/pull/25/files)
そうするとwhileの中の実装が簡潔になるだけでなく、22~30行目もいらなくなる気がします。

@nittoco
レビューありがとうございます。
左右いずれかのnodeがnullptrであってもqueueに入れてwhileの中で処理する方式にてnew_node_step2.cppに実装しました。
こちらの方が少し簡潔に書けました。

nittoco · 2024-08-01T09:38:15Z

617.MergeTwoBinaryTrees/new_node.cpp

+    TreeNode* new_root = new TreeNode(node1->val + node2->val);
+
+    queue<Nodes> search_nodes;
+    search_nodes.emplace(new_root, node1, node2);


タプルのようなものを利用して、new_root, node1, node2を1つにまとめて突っ込んだ方がわかりやすいかなと思いました。

今回はstructを使って指摘の三つを一緒に管理するようにしてみました。
struct Nodes { TreeNode* current_node; TreeNode* first_node; TreeNode* second_node; };
c++ではタプルより構造体を使おうとのことでしたのでstructにしました🙇
https://google.github.io/styleguide/cppguide.html#Structs_vs._Tuples

なるほど、知らなかったですありがとうございます

liquo-rice · 2024-08-01T11:38:22Z

617.MergeTwoBinaryTrees/new_node.cpp

+ */
+class Solution {
+private:
+  struct Nodes {


このstructを作ることで特にわかりやすくなっていないように感じます。

@liquo-rice
ここなのですがstructを使わないでpairなどに入れる、structの中身を親の状態を保持するTreeNodeと左右を保持するpairにするなど選択肢があるのかなと思いましたがどれが分かりやすくなっているのか判断できませんでした。

元々実装でstructを使うことで親と左右のNodeを一緒に格納できて良いなと思っておりました。

メンバー変数名がイマイチなように感じました。

@liquo-rice
メンバー変数名を更新しました。確かに指摘の通りfirstとsecondはよくないですね。
修正後はnode、 left_child、right_childです。nodeはparent_nodeとかにしようと思いましたが単純にnodeとしました。
cd017d4

liquo-rice · 2024-08-01T11:40:12Z

617.MergeTwoBinaryTrees/new_node.cpp

+ * };
+ */
+class Solution {
+private:


privateはpublicのあとに書いたらいいでしょう。https://google.github.io/styleguide/cppguide.html#Declaration_Order

@liquo-rice
一旦structを使ったもので、privateの位置を変えました。
定義についての順序も気をつけます。
de2aa9c

liquo-rice · 2024-08-01T11:41:13Z

617.MergeTwoBinaryTrees/memo.md

@@ -0,0 +1,44 @@
+## ステップ1


まず再帰で書いてみるのが自然な気がします。

liquo-rice · 2024-08-01T11:42:16Z

617.MergeTwoBinaryTrees/memo.md

+O(n)
+
+新しくnodeを作る方法
+TreeNodeクラスがデストラクタをどのように定義しているのか気になるところ


TreeNodeのdtorはtrivialだと思います。

@liquo-rice
いつもありがとうございます。

A trivial destructor is a destructor that performs no action.

とございました。https://en.cppreference.com/w/cpp/language/destructor
ということは何かしら確保したリソースをdeleteする必要があるということでしょうか？

LeetCodeのTreeNodeでは、dtorが定義されていないので、defaut dtorがコンバイラによって追加されています。

@liquo-rice
すみません参照した記事のすぐ下に挙動が書いてありました。失礼しました。

A trivial destructor is a destructor that performs no action. Objects with trivial destructors don't require a delete-expression and may be disposed of by simply deallocating their storage. Al

liquo-rice · 2024-08-01T11:43:42Z

617.MergeTwoBinaryTrees/new_node.cpp

+    search_nodes.emplace(new_root, node1, node2);
+
+    while (!search_nodes.empty()) {
+      const auto [current_node, first_node, second_node] = std::move(search_nodes.front());


struct Nodesの場合、move constructorもcopy constructorも同じ処理になると思います。

やはりなんとなくでしか理解していないようなので、忘れていたスクラッチでのvectorを実装してmoveの動作確認してみます。

@liquo-rice
指摘されていたのは以下のようなためでしょうか。

For user-defined types, the copy behavior is defined by the copy constructor and the copy-assignment operator. Move behavior is defined by the move constructor and the move-assignment operator, if they exist, or by the copy constructor and the copy-assignment operator otherwise.

https://google.github.io/styleguide/cppguide.html#Copyable_Movable_Types
自分で定義しないとコピーが行われるということですね。

default move ctorはメンバー変数ごとにmoveが行われます。intやポインタなどbuilt-in型の変数の場合は、単なる代入になります。

fhiyo · 2024-08-01T14:35:31Z

617.MergeTwoBinaryTrees/memo.md

+新しく作るという手段は思いついたけど、入力と分けて管理する必要性や新しくnodeを管理するコストを考えて
+片方に追加していく方式にした。


新しく作ることのメリット、入力を壊すデメリットについても言及しておいてもよいかもしれません

@fhiyo ありがとうございます。
fhiyo/leetcode#25
をもういちど拝見して下記に目を通しました🙇

入力を破壊している。
https://discord.com/channels/1084280443945353267/1252267683731345438/1252556045524537344
出力を変更されるとキャッシュが変わって次からの呼び出しが狂う。
https://discord.com/channels/1084280443945353267/1252267683731345438/1252591437485441024

fhiyo · 2024-08-01T14:38:45Z

617.MergeTwoBinaryTrees/step3.cpp

+    TreeNode* second_node;
+  };
+
+  void MergeOrPushQueue(queue<TwoNodes> &search_nodes, TreeNode*& first_node, TreeNode*& second_node) {


second_nodeの型は TreeNode* const でもいい気がします。

@fhiyo
コメントありがとうございます。
second_nodeだけconstをつけるとfirst_nodeにマージする際にconst有無の不一致でエラーとなりました🙇

それって、 const TreeNode* にしているからではないですか？

@fhiyo
早速ありがとうございます。
ご指摘の通りでした。2つの書き方の違いも調べました。今回は、ポインタが指すノードを固定するようにポインタ自体を書き換えられないようにするのですね。

関係するのはこの辺ですかね: https://stackoverflow.com/questions/1143262/what-is-the-difference-between-const-int-const-int-const-and-int-const

@fhiyo
資料までありがとうございます。読んでみます！！

ポインターの参照は、私は軽い抵抗感がありますが、これは、Google のスタイルガイドが、大きなオブジェクトの引数は const 参照かポインターかにして、呼び出し元から書き換えられる可能性のある変数を分かるようにするというルールを敷いていたからだと思います。

あと、型の宣言の方法は一回調べておいてください。いくらでも複雑な型が作れるのでルールを理解しておいたほうがいいです。

@oda

大きなオブジェクトの引数は const 参照かポインターかにして、呼び出し元から書き換えられる可能性のある変数を分かるようにするというルールを敷いていたから

この辺りでしょうか。
https://google.github.io/styleguide/cppguide.html#Use_of_const

宣言のあたりは読んだことがなかったので、型の部分に関しては以下を読んでみました。
https://google.github.io/styleguide/cppguide.html#Classes
https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#c-classes-and-class-hierarchies

TORUS0818 · 2024-08-02T06:14:22Z

拝見しました。
コメントされている部分以外は良いと思います。

kazukiii · 2024-08-02T23:33:22Z

617.MergeTwoBinaryTrees/new_node.cpp

+      TreeNode* first_left = nullptr;
+      TreeNode* second_left = nullptr;
+      if (first_node) {
+        first_left = first_node->left;


基本新しくノードを作っているが、ここは元々のノードを再利用しているところが、なにか一貫性のなさを感じました。

Ryotaro25 · 2024-08-05T01:49:46Z

拝見しました。コメントされている部分以外は良いと思います。

@TORUS0818
レビューありがとうございます。

finish

b670187

nittoco reviewed Aug 1, 2024

View reviewed changes

liquo-rice reviewed Aug 1, 2024

View reviewed changes

fhiyo reviewed Aug 1, 2024

View reviewed changes

kazukiii reviewed Aug 2, 2024

View reviewed changes

Ryotaro25 added 2 commits August 5, 2024 12:29

add modified files

de2aa9c

modifyr member names

cd017d4

colorbox mentioned this pull request Jan 26, 2025

617. Merge Two Binary Trees colorbox/leetcode#37

Merged

finish

4a9373e

		新しく作るという手段は思いついたけど、入力と分けて管理する必要性や新しくnodeを管理するコストを考えて
		片方に追加していく方式にした。

Conversation

Ryotaro25 commented Aug 1, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Ryotaro25 Aug 5, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Ryotaro25 Aug 5, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

TORUS0818 commented Aug 2, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Ryotaro25 commented Aug 5, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

Ryotaro25 Aug 5, 2024 •

edited

Loading

Ryotaro25 Aug 5, 2024 •

edited

Loading