Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions Arai60/27. Minimum Depth of Binary Tree/Makefile
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
NAME = a.out
SRC = main.cpp
CXX = g++
CXXFLAGS = -Wall -Wextra -Werror -std=c++11

all: $(NAME)

$(NAME): $(SRC) step1.hpp
$(CXX) -o $(NAME) $(SRC) $(CXXFLAGS)

run: $(NAME)
./$(NAME)
63 changes: 63 additions & 0 deletions Arai60/27. Minimum Depth of Binary Tree/main.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
#include "step3.hpp"
#include <iostream>
#include <queue>
Comment on lines +1 to +3

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

ヘッダーの順番は、標準ライブラリーが上のことが多いと思いましたが、これは、step3.hpp をテストするから一番上なんですかね。

https://google.github.io/styleguide/cppguide.html#Names_and_Order_of_Includes

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

今回はclang-formatを使用しなかったためでした。
この辺のヘッダーの並び替えは普段は大体clang-formatに任せているので以前に調べたっきり、ぼんやりとしか意識していませんでした。

ところで、改めて読んでみると Relative headerを最初に載せるということが書いてあってそれは知りませんでした。今回の場合ですと、名前はmain.cppになっているのであまり良くない気がしますが、もしこれがtest_step3.cppとかであれば、この順番でもいいんでしょうか。

Include headers in the following order: Related header, C system headers, C++ standard library headers, other libraries' headers, your project's headers.


#define null 9999999

TreeNode *buildTree(std::initializer_list<int> values) {
if (values.size() == 0) {
return nullptr;
}
TreeNode *root = new TreeNode(*values.begin());
std::queue<TreeNode *> nodes;
nodes.push(root);
for (size_t i = 1; i < values.size(); ++i) {
auto node = nodes.front();
nodes.pop();
int val1 = *(values.begin() + i);
int val2 = *(values.begin() + ++i);
if (val1 != null) {
node->left = new TreeNode(val1);
nodes.push(node->left);
}
if (val2 != null) {
node->right = new TreeNode(val2);
nodes.push(node->right);
}
}
return root;
}

void printTestCase(std::initializer_list<int> values) {
std::cout << "Test case: [";
for (auto it = values.begin(); it != values.end(); ++it) {
if (*it == null) {
std::cout << "null";
} else {
std::cout << *it;
}
if (it != values.end() - 1) {
std::cout << ",";
}
}
std::cout << "]" << std::endl;
}

void test(std::initializer_list<int> values, int expected) {
TreeNode *root = buildTree(values);
printTestCase(values);
Solution solution;
int result = solution.minDepth(root);
if (result == expected) {
std::cout << "PASSED" << std::endl;
} else {
std::cout << "FAILED" << std::endl;
}
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

正直、テストコードのテストコードが欲しくなりますね。
テストはできるだけ、繰り返しが多くても単純にしたいので、下のようなのを作りますかね。

TreeNode* node =
    new TreeNode(
        0,
        new TreeNode(
            1,
            new TreeNode(
                3
            ),
            new TreeNode(
                4
            )
        ),
        new TreeNode(
            2
        )
    );

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

正直、テストコードのテストコードが欲しくなりますね。

そうですよね。

テストケースの数だけ、こういう風に書くイメージですか?

  test(nullptr, 0);
  test(new TreeNode(1), 1);
  test(new TreeNode(3,
                    new TreeNode(9),
                    new TreeNode(20,
                                 new TreeNode(15),
                                 new TreeNode(7))),
       2);
  test({2, null, 3, null, 4, null, 5, null, 6}, 5);
  test(new TreeNode(2, nullptr,
                    new TreeNode(3, nullptr,
                                 new TreeNode(4, nullptr,
                                              new TreeNode(5, nullptr,
                                                           new TreeNode(6))))),
       5);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

実際は、テストフレームワークをなんらか使うはずで、そうなると大体の場合は、テストケース一つにつき関数を一つ作ることになるでしょう。また、再帰的な delete は別にすることになるでしょう。Address Sanitizer のような終了時のメモリー状態を確認する機能が動作するようにするためです。
https://google.github.io/googletest/gmock_cook_book.html

int main() {
test({}, 0);
test({1}, 1);
test({3, 9, 20, null, null, 15, 7}, 2);
test({2, null, 3, null, 4, null, 5, null, 6}, 5);
}
59 changes: 59 additions & 0 deletions Arai60/27. Minimum Depth of Binary Tree/step1.hpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
/*
何がわからなかったか
- N/A
何を考えて解いていたか
1. min depthを求めるので、BFSを使いたい
正解してから気づいたこと
- 最後のunreachableなcodeについて、どうしたらいいか
std::unreachableはC++23から使えるらしいが、新しすぎるか
https://en.cppreference.com/w/cpp/utility/unreachable
__builtin_unreachable()を使うのが良いかもしれないが、それもどうか
- depthはqueueに入れてしまった方が見通しが良いかも
*/
#ifndef STEP1_HPP
#define STEP1_HPP
/**
* Definition for a binary tree node.
*/
struct TreeNode {
int val;
TreeNode *left;
TreeNode *right;
TreeNode() : val(0), left(nullptr), right(nullptr) {}
TreeNode(int x) : val(x), left(nullptr), right(nullptr) {}
TreeNode(int x, TreeNode *left, TreeNode *right)
: val(x), left(left), right(right) {}
};

#include <queue>

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

これは struct TreeNode の上を勧めます。


class Solution {
public:
int minDepth(TreeNode *root) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

これは、これでいいですが、大規模開発の場合は分割コンパイルをします。
その時には、ヘッダーファイルには
int minDepth(TreeNode *root);
と宣言だけして、
.cpp に
int Solution::minDepth(TreeNode *root) {
}
本体を書きます。

if (root == nullptr)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

{}のないifブロックはちょっとした編集ミスでif条件の位置がズレてバグるので、可能であれば{}で囲うことをおすすめします。
https://ttsuki.github.io/styleguide/cppguide.ja.html#Formatting_Looping_Branching

return 0;
std::queue<TreeNode *> visitQueue;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

個人的にはローカルの変数名や引数は lower_snake にすることが多いですが、所属するチームの平均的な書き方に合わせれば問題ないと思います。

int depth = 1;
visitQueue.push(root);
while (!visitQueue.empty()) {
int numInThisDepth = visitQueue.size();
for (int i = 0; i < numInThisDepth; ++i) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

numInThisDepth 個だけキューの要素を処理するという書き方は、個人的にはややもやっとします。 2 つの vector を用意し、交互に使用する感じのほうが、個人的にはすっきりしていると思います。

std::vector<TreeNode *> nodes;
int depth = 1;
nodes.push_back(root);
while (!nodes.empty()) {
  std::vector<TreeNode *> nodesInNextDepth;
  for (auto node : nodes) {
    if (node->left == nullptr && node->right == nullptr) {
      return depth;
    }
    if (node->left) {
      nodesInNextDepth.push_back(node->left);
    }
    if (node->right) {
      nodesInNextDepth.push_back(node->right);
    }
  }
  std::swap(nodes, nodesInNextDepth);
  ++depth;
}

個人の好みの問題かもしれません。

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

ありがとうございます。

auto node = visitQueue.front();
visitQueue.pop();
if (node->left == nullptr && node->right == nullptr) {
return depth;
}
if (node->left) {
visitQueue.push(node->left);
}
if (node->right) {
visitQueue.push(node->right);
}
}
++depth;
}
// This should never be reached
return depth;
}
};
#endif // STEP1_HPP
89 changes: 89 additions & 0 deletions Arai60/27. Minimum Depth of Binary Tree/step2.hpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,89 @@
/*
講師陣はどのようなコメントを残すだろうか?
- コードの移植性について
他の人のコードを読んで考えたこと
- https://github.com/colorbox/leetcode/pull/36
pushの代わりにemplaceを使っているが、どう違うんだっけ
- (!ptr)と(ptr == nullptr)は
郷に行っては郷に従えスタンスだけど、自分で書く場合はいつも迷う。
個人的には前者は短いけど分かりづらく、後者好きだけど長くなる。

改善する時にかんがえたこと
- そもそもgcc/clang以外でコンパイルしたことがないから他のコンパイラ(主にMSVC?)のためにどれくらいポータブルなコードを書くべきなのかがよくわかっていない。
(std::unreachableはC++23からで新しすぎるかな)
https://en.cppreference.com/w/cpp/utility/unreachable
- なるほど、新しく値をコンストラクトしてpushするとcopy/moveしてしまうので、
コンストラクタの引数をemplaceに渡す方が効率が良いということか。
https://en.cppreference.com/w/cpp/container/queue/emplace
https://stackoverflow.com/questions/26198350/c-stacks-push-vs-emplace
- DFSで書くときには、当初leftかrightが0の場合を考慮せずreturnしてしまった
なんか考慮漏れがあってスッと書けない感じがした

*/
#ifndef STEP2_HPP
#define STEP2_HPP
/**
* Definition for a binary tree node.
*/
struct TreeNode {
int val;
TreeNode *left;
TreeNode *right;
TreeNode() : val(0), left(nullptr), right(nullptr) {}
TreeNode(int x) : val(x), left(nullptr), right(nullptr) {}
TreeNode(int x, TreeNode *left, TreeNode *right) : val(x), left(left), right(right) {}
};

#include <queue>
#include <utility>

class Solution {
public:
// 1. depthをqueueに
// 2. __builtin_unreachable()を使ってみた(gcc/clangのみだが)
// 3. pushの代わりにemplaceを使ってみた
int minDepth(TreeNode* root) {
if (root == nullptr) return 0;
std::queue<std::tuple<TreeNode *, int>> visitQueue;
visitQueue.emplace(root, 1);
while (!visitQueue.empty()) {
auto [node, depth] = visitQueue.front();
visitQueue.pop();
if (node->left == nullptr && node->right == nullptr) { return depth; }
if (node->left) { visitQueue.emplace(node->left, depth + 1); }
if (node->right) { visitQueue.emplace(node->right, depth + 1); }
}
__builtin_unreachable();
}

// root == nullptrを特別扱いせずにdummyを使ってみる
int minDepth2(TreeNode* root) {
TreeNode dummy(0, root, nullptr);
std::queue<std::tuple<TreeNode *, int>> visitQueue;
visitQueue.emplace(&dummy, 0);
while (!visitQueue.empty()) {
auto [node, depth] = visitQueue.front();
visitQueue.pop();
if (node->left == nullptr && node->right == nullptr) { return depth; }
if (node->left) { visitQueue.emplace(node->left, depth + 1); }
if (node->right) { visitQueue.emplace(node->right, depth + 1); }
}
__builtin_unreachable();
}

// DFSでもやってみる
// どちらにせよO(N)だが平衡に近い木ではだいぶ不利なはず
int minDepth3(TreeNode* root) {
auto dfs = [&](auto &&self, TreeNode *node) -> int {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

dfsという命名に関しては、discordで議論がいくつかあるので見てみてください

if (node == nullptr) { return 0; }
int left = self(self, node->left);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

left, rightは、left_depth, right_depthと、深さを表すことを示した方がわかりやすそうです。
TreeNode構造体の要素の名称と重なるのはやや抵抗感があります

int right = self(self, node->right);
if (left == 0) { return 1 + right; }
if (right == 0) { return 1 + left; }
return 1 + std::min(left, right);
};
return dfs(dfs, root);
}
};
#endif // STEP2_HPP

37 changes: 37 additions & 0 deletions Arai60/27. Minimum Depth of Binary Tree/step3.hpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
/*
3問連続で正解するのにかかった時間 10分
時間計算量: O(N)
空間計算量: O(N)
*/

#ifndef STEP3_HPP
#define STEP3_HPP

#include <queue>

struct TreeNode {
int val;
TreeNode *left;
TreeNode *right;
TreeNode() : val(0), left(nullptr), right(nullptr) {}
TreeNode(int x) : val(x), left(nullptr), right(nullptr) {}
TreeNode(int x, TreeNode *left, TreeNode *right): val(x), left(left), right(right) {}
};

class Solution {
public:
int minDepth(TreeNode *root) {
if (root == nullptr) return 0;
std::queue<std::tuple<TreeNode*, int>> visitQueue;

@t0hsumi t0hsumi Feb 4, 2025

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Queueであることは、型名から自明なのでなくて良いと思います。
TreeNodeとintが何を表しているか明確にするために、nodes_and_depthsとかを使っているコードをよくみます。

個人的には、小さい関数で関数名から何をするか大体わかるし、c++なら型を明記できるので、next_visitとか中身を明記しない変数名でもいいとは思います。

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

ご指摘いただいた点、確かに私も同感です。

nodes_and_depthsという名称は、変数に格納される要素(ノードと深さのペア)の型情報は明示しているものの、アルゴリズム上の役割、つまり「これから訪問するべきノード群」という意図を十分に伝えられていないと感じました。一方、next_visitでは単一の要素を連想させてしまうため、どちらも意図を正確に表現できていないと考えています。

そのため、たとえばnodesToVisitpendingVisits、またはBFSの文脈でよく用いられるfrontierなど、複数形や集合を示唆する名称の方が意図を正確に伝えられるかと思います。

今後の実装では、こうした名称を検討してみたいと思います。

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

また、やや今回のケースではやりすぎな感もありますが、このように型情報として中に含まれているのがdepthであることを明示するのも一つの手かもしれません。

typedef int depth_t;
std::queue<std::tuple<TreeNode*, depth_t>> nodesToVisit;

visitQueue.emplace(root, 1);
while (!visitQueue.empty()) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

while (1) にしてしまうのは一つだと思います。

auto [node, depth] = visitQueue.front();
visitQueue.pop();
if (node->left == nullptr && node->right == nullptr) return depth;
if (node->left) visitQueue.emplace(node->left, depth + 1);
if (node->right) visitQueue.emplace(node->right, depth + 1);
}
__builtin_unreachable();
}
};
#endif