102. Binary Tree Level Order Traversal #27
Conversation
| vector<vector<int>> levelOrder(TreeNode* root) { | ||
| if (!root) return vector<vector<int>>(); | ||
| vector<vector<int>> answer; | ||
| queue<TreeNode*> nodes_to_visit; |
There was a problem hiding this comment.
step2_bfs_two_vectors のように、レベルごとに vector を作ったほうが処理が分かりやすいと思いました。
また、変数名の to_visit の部分は、有益な情報が含まれていないように感じました。 nodes で十分だと思います。
There was a problem hiding this comment.
ありがとうございます。レベルごとに vector を作る実装を第一選択肢として考えるようにしてみます。
個人的には to_visit があった方が、これから探索するノードということがはっきりすると思いますがいかがでしょうか？
nodes だけだとこれはどんなノードなんだとなりそうな気がします。
| queue<TreeNode*> nodes_to_visit; | ||
| nodes_to_visit.push(root); | ||
| while (!nodes_to_visit.empty()) { | ||
| int level_size = nodes_to_visit.size(); |
There was a problem hiding this comment.
level_size という変数名ですと、レベルのサイズという意味に感じられ、何を表しているのか分かりにくく感じました。 num_nodes_in_level はいかがでしょうか？
There was a problem hiding this comment.
num_nodes_in_level いいですね。step4で修正します。
| nodes_to_visit.push(root); | ||
| while (!nodes_to_visit.empty()) { | ||
| int level_size = nodes_to_visit.size(); | ||
| vector<int> current_level_values; |
There was a problem hiding this comment.
個人的には values_in_level としたいですが、 current_level_values でも acceptable だと思います。
| if (node->right) next_level_nodes.push(node->right); | ||
| } | ||
| answer.push_back(move(current_level_values)); | ||
| nodes_to_visit = next_level_nodes; |
There was a problem hiding this comment.
= だとコピーが走ります。 std::swap() したほうが処理が軽くなると思います。
There was a problem hiding this comment.
コピー走りますね。。まだ意識が甘いようです、ありがとうございます。
std::swap, std::vector::swap ともにC++98から使用可能で、std::swap は内部で std::vector::swap を呼び出すようなのでどちらを使っても良さそうです。
Swaps the contents of lhs and rhs. Calls lhs.swap(rhs).
https://en.cppreference.com/w/cpp/container/vector/swap
https://en.cppreference.com/w/cpp/container/vector/swap2
| vector<vector<int>> levelOrder(TreeNode* root) { | ||
| if (!root) return vector<vector<int>>(); | ||
| vector<vector<int>> answer; | ||
| queue<TreeNode*> nodes_to_visit; |
There was a problem hiding this comment.
一つのキューで順不同で push() と pop() をしたりするわけではく、先頭から順にノードを舐められれば十分なため、 queue にする必要が無いように感じました。 step2_bfs_two_vectors のように、 vector 2 つで十分だと思います。
| while (!nodes_to_visit.empty()) { | ||
| auto [node, level] = nodes_to_visit.top(); | ||
| nodes_to_visit.pop(); | ||
| if (answer.size() == level) { |
There was a problem hiding this comment.
answer に push_back() する処理を省くため、 map<int, vector> level_to_nodes; として、最後に answer に詰め替えるのも良いかなと思ったのですが、処理がやや重くなるのが微妙にも感じました。
| int level_size = nodes_to_visit.size(); | ||
| for (int i = 0; i < level_size; i++) { |
There was a problem hiding this comment.
while (!nodes_to_visit.empty()) { のように書いてもいいですね。
こちらの場合nodes_to_visitが空になるまで処理を行うことが明確になる気はします
There was a problem hiding this comment.
たしかに while (!nodes_to_visit.empty()) { と書いた方が一目でわかりますね。ありがとうございます。
| while (!nodes_to_visit.empty()) { | ||
| auto [node, level] = nodes_to_visit.top(); | ||
| nodes_to_visit.pop(); | ||
| if (answer.size() == level) { |
There was a problem hiding this comment.
一応、こういう議論があったので貼っておきます
https://discord.com/channels/1084280443945353267/1200089668901937312/1211248049884499988
There was a problem hiding this comment.
なるほど。この考えはありませんでした。
たしかに if より while の方が読んでる側からすると安心感がありますね。
| nodes_to_visit.push(root); | ||
| while (!nodes_to_visit.empty()) { | ||
| int level_size = nodes_to_visit.size(); | ||
| vector<int> level_values; |
There was a problem hiding this comment.
これで良さそうですかね。
auto& level_values = answer.emplace_back();
There was a problem hiding this comment.
この書き方は思いつきませんでした。勉強になります、ありがとうございます。
問題へのリンク
https://leetcode.com/problems/binary-tree-level-order-traversal/description/
次に解く問題
103. Binary Tree Zigzag Level Order Traversal
README.mdへ頭の中の言語化と記録をしています。