102. Binary Tree Level Order Traversal by Ryotaro25 · Pull Request #28 · Ryotaro25/leetcode_first60

Ryotaro25 · 2024-08-21T15:23:34Z

問題へのリンク
https://leetcode.com/problems/binary-tree-level-order-traversal/description/
問題文(プレミアムの場合)

備考

次に解く問題の予告
Binary Tree Zigzag Level Order Traversal

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

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

fhiyo · 2024-08-21T16:10:17Z

102.BinaryTreeLevelOrderTraversal/step3.cpp

+  vector<vector<int>> levelOrder(TreeNode* root) {
+    vector<vector<int>> level_to_values = {};
+
+    queue<TreeNode*> smae_level_nodes;


smaeとは何でしょうか？typoかと思ったんですがstep2でもあったので

@fhiyo
レビューありがとうございます。
サジェスト(補完)を使っていたので気づかないまま流用しておりました。
失礼しました。

fhiyo · 2024-08-21T16:15:17Z

102.BinaryTreeLevelOrderTraversal/dfs.cpp

+    if (!root) {
+      return {};
+    }


これ不要かなと思います

@fhiyo
ご指摘のとおり不要でした。修正しました。

fhiyo · 2024-08-21T16:22:54Z

102.BinaryTreeLevelOrderTraversal/memo.md

+O(n)
+
+## ステップ2
+・queueに入れてから、nodeが有効なのか判断する方に変更


あくまで個人的ですが、自分はqueueに入れる前に判断する方が好きです。
ゴミかもしれないけどとりあえずqueueに入れて後で調整する、という方法だとちょっと認知負荷がある気がしたので...今回だと if (!values.empty()) { ... } の条件文を自分だと忘れそうだなと思いました

確かにこれまでの類似問題と比較しても、if (!values.empty()) { ... }は特別に処理を入れてますね。
yoshikiさんからのレビュー内容も踏まえて、queue管理をvectorに変更しvectorに入れる前に判断する方式に変更しました。ありがとうございます。

Yoshiki-Iwasa · 2024-08-21T21:44:26Z

102.BinaryTreeLevelOrderTraversal/step3.cpp

+    while (!same_level_nodes.empty()) {
+      int nodes_num = same_level_nodes.size();
+      vector<int> values = {};
+      for (int i = 0; i < nodes_num; i++) {


same_level_nodesって命名微妙かなと思います
このループのなかで次のレベルのノードも入ってるので

same_level_nodesは単純なqueueにしちゃってsame_level_nodes,next_level_nodesそれぞれの配列を逐一用意してあげたほうが読みやすくなるのではと思いました

@Yoshiki-Iwasa
レビューありがとうございます。
現在と次の2種のnodeが入っているので変数名正しく無いですね。それぞれを別管理にする方式に変更してみました。
step4.cppとなります。

97045e7

一般に、一つの変数に二つの役割を持たすのはいまいちな気がします(自分もよくやってしまいますが、、、）

@nittoco
レビューありがとうございます。
これは意識して気をつけようと思います🙇

liquo-rice · 2024-08-22T18:35:01Z

102.BinaryTreeLevelOrderTraversal/step4.cpp

+        }
+      }
+      level_to_values.emplace_back(values);
+      current_level_nodes = next_level_nodes;


copy assignmentが呼ばれそうです

@liquo-rice
レビューありがとうございます。こちらに処理を追加したファイルを上げました(step5.cpp)。
copy/move周りを理解するために、前に教えていただいたvectorを自前で作ってcopy/moveにconstructor/assignmentを実装してみました(vector.cpp)。

メモにポイントを纏めてみましたので認識に誤りございましたら指摘頂きたいです🙇
先延ばしにしておりましたがよろしくお願いいたします。
2eeaea0

返却値がvector<vector>であったため、既存のvectorに置き換えて自分の定義を使うことができませんでした。なのでクラスだけ作りました。

swap を使うのも一つです。

liquo-rice · 2024-08-22T18:35:40Z

102.BinaryTreeLevelOrderTraversal/step4.cpp

+          next_level_nodes.emplace_back(node->right);
+        }
+      }
+      level_to_values.emplace_back(values);


copy constructorが呼ばれそうです

liquo-rice · 2024-08-22T18:37:30Z

102.BinaryTreeLevelOrderTraversal/step4.cpp

+      vector<int> values = {};
+      vector<TreeNode*> next_level_nodes = {};
+
+      for (const auto& node : current_level_nodes) {


ポインタへの参照にする意味はなさそうです

確かにアドレスをさし示しているので意味ないですね。。。失礼しました。

& をつけるかは、サイズや変更したいかなどで決めましょう。

liquo-rice · 2024-08-22T18:38:21Z

102.BinaryTreeLevelOrderTraversal/step4.cpp

+class Solution {
+public:
+  vector<vector<int>> levelOrder(TreeNode* root) {
+    vector<vector<int>> level_to_values = {};


= {}は不要ですが、あってもいいかもしれないです

@liquo-rice
effective c++の４項を読んで自分でデフォルトの動きを覚えるより、オブジェクトは初期化を必ず行う方が今の自分にはいいと思いました。
「vectorはいつでも初期化されると保証されている」とは記載ございました。

kazukiii · 2024-08-24T02:06:29Z

102.BinaryTreeLevelOrderTraversal/step1.cpp

+    }
+    map<int, vector<int>> level_to_values;
+
+    queue<NodeAndDepth> node_and_depth;


細かいですが複数形にすると良さそうです。node_and_depths

kazukiii · 2024-08-24T02:10:35Z

102.BinaryTreeLevelOrderTraversal/step1.cpp

+
+    vector<vector<int>> values;
+    for (const auto& [depth, vals] : level_to_values) {
+      values.emplace_back(vals);


コピーが走っていそうです。またこの場合その場でオブジェクトを構築している訳ではないので、push_backでも同じ動きになると思います。

@kazukiii
レビューありがとうございます。
左辺値と右辺値で挙動が変わるのは理解しておりませんでした。ありがとうございます。
https://en.cppreference.com/w/cpp/container/vector/push_back

こちらに処理を追加したファイルを上げました(step5.cpp)。
この辺りを理解するために、前に教えていただいたvectorを自前で作ってcopy/moveにconstructor/assignmentを実装してみました(vector.cpp)。また以前いただいた資料も再度読みました。
2eeaea0

kazukiii · 2024-08-24T02:12:17Z

102.BinaryTreeLevelOrderTraversal/step2.cpp

+    queue<TreeNode*> same_level_nodes;
+    same_level_nodes.emplace(root);
+    while (!same_level_nodes.empty()) {
+      int nodes_num = same_level_nodes.size();


英字句にすると number of nodes になると思うので、num_nodesの方が良さそうです。

kazukiii · 2024-08-24T02:13:45Z

102.BinaryTreeLevelOrderTraversal/step2.cpp

+        same_level_nodes.emplace(node->right);
+      }
+
+      if (!values.empty()) {


キューに nullptr も入れていることによってこの辺りが逆に複雑になっているような気がします。

kazukiii · 2024-08-24T02:18:30Z

102.BinaryTreeLevelOrderTraversal/step4.cpp

+
+    vector<TreeNode*> current_level_nodes = {root};
+    while (!current_level_nodes.empty()) {
+      vector<int> values = {};


C++では参照を受け取ってそこへ直接書き込むことも出来ますね。ご参考までに。
vector<int>& values = level_to_values.emplace_back();

ありがとうございます！

nittoco · 2024-08-24T13:34:16Z

見ました！
moveと右辺値参照のところ勉強になりました🙏

liquo-rice · 2024-08-24T17:05:44Z

102.BinaryTreeLevelOrderTraversal/vector.cpp

+  }
+
+  // move コンストラクター
+  MyVector(MyVector&& r) : arr(new T[r.capacity]), capacity(r.capacity), current(r.current) {


move ctorとinsertの実装に問題がありそうです。見直してみてください。

@liquo-rice
ご確認頂きありがとうございます。
再度自分のコードを読み直しました。

move ctor
moveであるのみnewであったりコピーをしていることが問題だと思いました。
なので初期化子演算子内でnewを使わないようにして、forループの部分は削除しました。

insert
インデックスがマイナスなど、不正なインデックスに対してハンドリングがなかったです。
インデックスが0より下もしくはキャパ以上の場合は例外を投げるようにしました。

250448d

最後の後ろに追加する場合以外は、単なる代入になっているようです。
https://en.cppreference.com/w/cpp/container/vector/insert

@liquo-rice

If after the operation the new size() is greater than old capacity() a reallocation takes place

現在のサイズを超える場合は、動的にサイズを変更させる必要ございましだ🙇
push_back内に定義していたサイズ変更処理を外出ししてresize()関数を作りました。

Linear in count plus linear in the distance between pos and end of the container.

途中に挿入された場合、シフトする必要ございました。
こちらが修正したものになります。
abff5d6

liquo-rice · 2024-08-24T17:08:21Z

102.BinaryTreeLevelOrderTraversal/step5.cpp

+      vector<TreeNode*> next_level_nodes = {};
+
+      for (const auto node : current_level_nodes) {
+        values.emplace_back(std::move(node->val));


node->valの型はintなので、moveは不要です。

@liquo-rice
失礼しました。左辺値と右辺値の取り扱いに慣れるまでは、ドキュメントを確認しながら進めるようにします🙇

TORUS0818 · 2024-08-26T09:14:44Z

拝見しました。
コメント以外の部分は良いと思いました。

finish

69adafc

fhiyo reviewed Aug 21, 2024

View reviewed changes

mod var names

319a303

Yoshiki-Iwasa reviewed Aug 21, 2024

View reviewed changes

modified

97045e7

liquo-rice reviewed Aug 22, 2024

View reviewed changes

fhiyo mentioned this pull request Aug 23, 2024

103. Binary Tree Zigzag Level Order Traversal #29

Merged

kazukiii reviewed Aug 24, 2024

View reviewed changes

created vector by scratch

2eeaea0

liquo-rice reviewed Aug 24, 2024

View reviewed changes

modified my vector

250448d

mod insert function

abff5d6

colorbox mentioned this pull request Feb 18, 2025

102. Binary Tree Level Order Traversal colorbox/leetcode#40

Open

finish

9ffc2a5

Conversation

Ryotaro25 commented Aug 21, 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!

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!

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!

nittoco commented Aug 24, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Ryotaro25 Aug 25, 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!

Ryotaro25 Aug 28, 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!

TORUS0818 commented Aug 26, 2024

Uh oh!

Reviewers

Assignees

Labels

Ryotaro25 Aug 25, 2024 •

edited

Loading

Ryotaro25 Aug 28, 2024 •

edited

Loading