Skip to content

104. Maximum Depth of Binary Tree#23

Open
Ryotaro25 wants to merge 3 commits intomainfrom
problem21
Open

104. Maximum Depth of Binary Tree#23
Ryotaro25 wants to merge 3 commits intomainfrom
problem21

Conversation

@Ryotaro25
Copy link
Copy Markdown
Owner

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

備考

次に解く問題の予告
Minimum Depth of Binary Tree

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

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

}
int depth = MeasureDepth(root);
return depth;
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

この関数、単に return MeasureDepth(root); でよくないでしょうか。としたら、二つを一つにしてもいいのではないでしょうか。

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.

@oda
レビューありがとうございます。直近の問題のほとんどを関数化していたので、関数化してしまいましたが
liquoさんからも指摘あったように振る舞いは変わらないので分ける必要ないですね。
step4にて修正しました。
94e08f1

*/
class Solution {
public:
int MeasureDepth(TreeNode* node) {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

この関数を公開する必要がないように見えました。privateでいいのかなと思います。

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

メンバー変数使わないのと、引数のノードも変更しないので、以下のように書けます。

static int MeasureDepth(TreeNode* node)

また、maxDepthとMeasureDepthは、呼び出し側からすると完全に同じ振る舞いをします。一つにまとめて良さそうです。

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.

@fhiyo @liquo-rice
レビューありがとうございます。staticを使うことも覚えておきます。
指摘の通り分ける必要ないので今回は一つに纏めました。なのでMeasureDepth関数自体削除しました。

94e08f1

Comment on lines +25 to +27
if (root == nullptr) {
return 0;
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

root == nullptrを特別扱いする必要がないように見えます。

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.

@fhiyo
関数側でチェックしているので必要ないですね。今回は他の指摘であったことから関数自体削除しましたのでメイン処理側では残しました。

*/
class Solution {
public:
int MeasureDepth(TreeNode* node) {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

メンバー変数使わないのと、引数のノードも変更しないので、以下のように書けます。

static int MeasureDepth(TreeNode* node)

また、maxDepthとMeasureDepthは、呼び出し側からすると完全に同じ振る舞いをします。一つにまとめて良さそうです。

if (node == nullptr) {
continue;
}
max_depth = max(depth, max_depth);
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

必ず、depth >= max_depthではないですか?

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

私もここは常にdepthのほうが大きいと思います.

max_depth = 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.

@liquo-rice @fhiyo
常に更新されているので意味無かったですね。修正しました。

@Yoshiki-Iwasa
Copy link
Copy Markdown

指摘あるところ以外良さそうです

*/
class Solution {
public:
int maxDepth(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.

Leetcode の都合上変えられないので仕方ないですが、引数に渡される 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.

各関数呼び出しを考えると、そのnodeを根とするツリーのmaxDepthを求めているという意味ではありなのかなと思います

その場合、left_tree_max_depth, right_tree_max_depthと変数名をつけることが適当でしょうか(長いか)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

各関数呼び出しを考えると、そのnodeを根とするツリーのmaxDepthを求めているという意味ではありなのかなと思います

あーなるほどその発想はありませんでした。そう考えると確かに不自然ではないかもしれないです。

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_tree_max_depth, right_tree_max_depthと変数名をつけることが適当でしょうか(長いか)

Java ならまあありそうだなって感じの長さに思いました。 C++ だとどうなんでしょうね

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.

7 participants