Create Max Area of Island.md by irohafternoon · Pull Request #20 · irohafternoon/LeetCode

irohafternoon · 2025-04-14T14:04:11Z

This Problem
Max Area of Island
Next Ploblem
Number of Connected Components in an Undirected Graph

Fuminiton · 2025-04-15T12:10:22Z

695. Max Area of Island/Max Area of Island.md

+    static bool isUnvisitedIsland(int row, int column, int num_row, int num_column,
+                                    const std::vector<std::vector<int>>& grid,
+                                    const std::vector<std::vector<int>>& is_visited_island) {
+        if (!(0 <= row && row < num_row) ||


ここ、rowとcolumnは独立させてもいいと思います。

if文の条件が長いと、読み手が条件を追うときに、
覚えておかなければいけない量が長くなるといった考え方です。

コメントありがとうございます
個別にreturn falseをしたほうが読み手の負荷が小さいと気づきました

Fuminiton · 2025-04-15T12:13:25Z

695. Max Area of Island/Max Area of Island.md

+        if (is_visited_island[row][column]) {
+            return false;
+        }
+        if (grid[row][column] == kSea) {


kSeaと名前を付けているのいいと思います。
0,1だけだと分かりづらいですよね。

Fuminiton · 2025-04-15T12:15:52Z

695. Max Area of Island/Max Area of Island.md

+                                const std::vector<std::vector<int>>& grid,
+                                std::vector<std::vector<int>>& is_visited_island) {
+
+        is_visited_island[row][column] = 1;


この変数、true or falseで管理する、もしくは、1に意味を持たせるための何かしらの変数に置きたくなりました。

kTrue = 1などとしてもよかったかもしれないです

kVisited = 1 はいかがでしょうか？

ありがとうございます。kVisitedがすでに見たニュアンスがあって優れていると思いました

oda · 2025-04-15T14:20:25Z

695. Max Area of Island/Max Area of Island.md

+};
+```
+
+- union_findを8分で書けるようになった


ここまで十二分だと思います。

ついでなので書いておくと、C++ では分割コンパイルすることが多く、その場合、class 内のメンバー変数等の宣言と実装を分離します。クラスの定義をヘッダーファイルに記述し、メンバー関数の実装をソースファイルに書きます。

class A { public: int f(); }; int A::f() { return 0; }

ありがとうございます。
分離するとヘッダファイルとソースファイル、実行ファイルを分離すると、ソースファイルの変更を一斉に反映させたりできる利便性があるため分割コンパイルされると理解しました

nodchip · 2025-04-16T01:07:39Z

695. Max Area of Island/Max Area of Island.md

+    Position(int row, int column) : row(row), column(column) {}
+};
+
+class unionFind {


クラス名等の型名は UpperCamel で書くことが多いように思います。

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

Type names start with a capital letter and have a capital letter for each new word, with no underscores: MyExcitingClass, MyExcitingEnum.

ありがとうございます、ClassはUpperCamelで書くようにいたします

nodchip · 2025-04-16T01:08:35Z

695. Max Area of Island/Max Area of Island.md

+
+class unionFind {
+public:
+    unionFind(int num_row, int num_column) : num_row_(num_row), num_column_(num_column),


自分が num_ という接頭辞を使用する場合は、そのあとにくるものが複数存在する場合が多いため、複数形にして num_rows とすることが多いです。

number of の略なので、基本的に複数形を取るべきと気づきました。ありがとうございます

nodchip · 2025-04-16T01:09:00Z

695. Max Area of Island/Max Area of Island.md

+class unionFind {
+public:
+    unionFind(int num_row, int num_column) : num_row_(num_row), num_column_(num_column),
+                                            parents_(num_row_, std::vector<Position>(num_column_, Position{0,0})) {


インデントがずれているようです。

nodchip · 2025-04-16T01:40:07Z

695. Max Area of Island/Max Area of Island.md

+        }
+    }
+
+    Position find(Position position) {


クラスや構造体を関数の引数に取る場合は、 const 参照にすることが多いです。理由は、値渡しにするとコピーが発生するためです。

ただ、 sizeof(Position) が 8 のため、もしかしたら CPU のレジスター経由で渡されるかもしれないと思いました。

調べたところ、 Windows と Linux で呼び出し規約は異なるものの、どちらもレジスターで渡されていました。

https://learn.microsoft.com/ja-jp/cpp/build/x64-calling-convention?view=msvc-170

整数値を持つ左端の 4 つの位置にある引数は、左から右の順序で、それぞれ RCX、RDX、R8、および R9 で渡されます。

サイズが 8、16、32、または 64 ビットの構造体と共用体、および __m64 型は、同じサイズの整数であるかのように渡されます。

https://azelpg.gitlab.io/azsky2/note/prog/asm64/22_systemv.html

基本的に、構造体全体のデータを、メモリ上の 8 byte 単位で分割し、そこに含まれるメンバのタイプを、一つのタイプに分類して、結果として、一つの 8 byte 整数、または浮動小数点数として、通常の引数と同じように扱います。

実際に試したところ、

class Position { int row; int column; }; void Hoge(Position position) { (void)position; } int main() { Position position; Hoge(position); return 0; }

Hoge(Position): push rbp mov rbp, rsp mov QWORD PTR [rbp-8], rdi // CPU の rdi には第 1 引数が格納されている。rdi レジスターの内容をスタックメモリにコピーしている。 nop pop rbp ret main: push rbp mov rbp, rsp sub rsp, 16 mov rax, QWORD PTR [rbp-8] // スタックメモリ上の構造体の内容を CPU の rax レジスターにコピーしている mov rdi, rax // rax レジスターの内容を rdi レジスターにコピーしている。これは関数の第 1 引数として扱われる。 call Hoge(Position) mov eax, 0 leave ret

結論としては、構造体のサイズが 1・2・4・8 バイトの場合は、引数に値渡しして良いということになりそうです。

ソフトウェアエンジニアの常識に入っているかどうかは微妙なラインだと思いました。

構造体のサイズは後々変わることがあるため const 参照が無難じゃないですかねー。

コメントと、実験結果をいただきありがとうございます。
知識が乏しく完全に理解はできていないと思うのですが、実際に8byteの構造体は、値渡しでもレジスタで幅に収まるので、レジスタ経由で渡されると理解しました。
ただ、const参照が無難とのことで、基本的にconst参照を使っていこうと思います

@nodchip
提示していただいたレジスタなどの動きを表示する方法について、私も実際に構造体のサイズを変えて実験してみたく、

g++ -S -O0 -masm=intel -std=c++20 test.cpp

のようにコンパイルすればファイルを作成できるというところまでは調べたのですが、野田さんの提示いただいたような形式とは少し違い、どのように生成したか教えていただくことは可能でしょうか？

自分はこちらのサイトで出力しました。
https://godbolt.org/
各種コンパイラーの各バージョンでソースコードをコンパイルした結果が出力されるサイトです。

ありがとうございます！
こちらで実験してみたいと思います

nodchip · 2025-04-16T01:41:20Z

695. Max Area of Island/Max Area of Island.md

+        }
+    }
+
+    Position find(Position position) {


C++ では関数名は UpperCamel で書くことが多いように思います。

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

Ordinarily, functions should start with a capital letter and have a capital letter for each new word.

ありがとうございます。今後は関数はUpperCamelで使うように統一しようと思います

nodchip · 2025-04-16T01:57:52Z

695. Max Area of Island/Max Area of Island.md

+                for (int i = 0; i < 4; i++) {
+                    auto next_r = r + kRowDiff[i];
+                    auto next_c = c + kColumnDiff[i];
+                    if (!(0 <= next_r && next_r < num_row) ||


個人的には
if (!(0 <= next_r && next_r < num_row && 0 <= next_c && next_c < num_column)) {
のほうが、目に入る要素数が少なくて読みやすいと思います。好みの問題かもしれません。

nodchip · 2025-04-16T01:59:13Z

695. Max Area of Island/Max Area of Island.md

+        parents_[parent2.row][parent2.column] = parent1;
+    }
+
+    int numElemntsOfMaxGroup () {


関数名は動詞の原形で始ま目ることが多いように思います。

ありがとうございます
少し長いですが、GetNumElementsOfMaxGroupのようにGetをつけるようにしようと思います

nodchip · 2025-04-16T02:01:34Z

695. Max Area of Island/Max Area of Island.md

+        parents_[parent2.row][parent2.column] = parent1;
+    }
+
+    int numElemntsOfMaxGroup () {


この関数は UnionFind の操作ではないため、 UnionFind クラスの中に入れるのは違和感を感じました。呼び出し元に書くか、呼び出し元の近くに独立した関数として書いたほうがよいと思います。

@nodchip ありがとうございます。
最大のグループのサイズを得るためには、UnionFindクラスのデータメンバであるsize_が必要で、UnionFind外で書くとsize_をprivateにできないと思い、UnionFind内で書かないといけないと思った次第です。
こちらのメソッドをUnionFindの外側で書く場合、データメンバのsize_を、publicにするということでしょうか。
もしくは、privateは保ったまま、UnionFind外でメソッドを書ける方法があるのでしょうか。

データメンバに size_ が見つかりませんでした。
find() の呼び出しだけで実装できると思いますので、現状の実装から numElemntsOfMaxGroup() 以外を変更することなく実装できると思いました。

STEP3でのUnionFindと混同しておりました、大変失礼いたしました。
以下のように分離できることを確認しました

#include <map> #include <vector> struct Position { int row; int column; auto operator<=>(const Position&) const = default; Position(int row, int column) : row(row), column(column) {} }; class UnionFind { public: UnionFind(int num_rows, int num_columns) : num_rows_(num_rows), num_columns_(num_columns), parents_(num_rows_, std::vector<Position>(num_columns_, Position{0,0})) { for (int r = 0; r < num_rows_; r++) { for (int c = 0; c < num_columns; c++) { parents_[r][c] = Position{r, c}; } } } Position find(Position position) { if (parents_[position.row][position.column] == position) { return position; } // 再帰的に親をたどって、経路圧縮を行う parents_[position.row][position.column] = find(parents_[position.row][position.column]); return parents_[position.row][position.column]; } //position2の親をposition1の親に繋ぐ void merge(Position position1, Position position2) { auto parent1 = find(position1); auto parent2 = find(position2); if (parent1 == parent2) { return; } parents_[parent2.row][parent2.column] = parent1; } private: const int num_rows_; const int num_columns_; std::vector<std::vector<Position>> parents_; }; int GetnumElemntsOfMaxGroup(UnionFind& uf, int num_rows, int num_columns) { std::map<Position, int> position_to_num_elements; for (int row = 0; row < num_rows; row++) { for (int column = 0; column < num_columns; column++) { auto parent = uf.find(Position{row, column}); position_to_num_elements[parent]++; } } auto max_size = 0; for (const auto& [_, num_elements] : position_to_num_elements) { if (num_elements > max_size) { max_size = num_elements; } } return max_size; } class Solution { public: int maxAreaOfIsland(const std::vector<std::vector<int>>& grid) { if (grid.empty() || isOnlySea(grid)) { return 0; } auto num_rows = grid.size(); auto num_columns = grid[0].size(); UnionFind island_union (num_rows, num_columns); for (int r = 0; r < num_rows; r++) { for (int c = 0; c < num_columns; c++) { if (grid[r][c] != kIsland) { continue; } for (int i = 0; i < 4; i++) { auto next_r = r + kRowDiff[i]; auto next_c = c + kColumnDiff[i]; if (!(0 <= next_r && next_r < num_rows) || !(0 <= next_c && next_c < num_columns)) { continue; } if (grid[next_r][next_c] != kIsland) { continue; } island_union.merge(Position{r, c}, Position{next_r, next_c}); } } } return GetnumElemntsOfMaxGroup(island_union, num_rows, num_columns); } private: static constexpr int kRowDiff[4] = {0, 0, 1, -1}; static constexpr int kColumnDiff[4] = {1, -1, 0, 0}; static constexpr int kIsland = 1; //UnionFindを用いると、各グループの最低要素数が1になるため、すべて海だとカウントを誤る //このため、事前にgridがすべて海かを確認する bool isOnlySea (const std::vector<std::vector<int>>& grid) { for (const auto& row : grid) { for (auto cell : row) { if (cell == kIsland) { return false; } } } return true; } };

nodchip · 2025-04-16T02:02:39Z

695. Max Area of Island/Max Area of Island.md

+    static constexpr int kRowDiff[4] = {0, 0, 1, -1};
+    static constexpr int kColumnDiff[4] = {1, -1, 0, 0};
+    static constexpr int kIsland = 1;
+    //unionFindを用いると、各グループの最低要素数が1になるため、すべて海だとカウントを誤る


あらかじめ island かどうかを判定し、 island についてのみ Find() を呼び出したほうがシンプルだと思いました。

ありがとうございます。イメージ的には、海マスsize_は0で、島マスのみUnionFindすることでこちらの関数を省略できると理解しました。

nodchip · 2025-04-16T02:04:36Z

695. Max Area of Island/Max Area of Island.md

+        auto max_island_size = 0;
+        for (int r = 0; r < num_row; r++) {
+            for (int c = 0; c < num_column; c++) {
+                if (isUnvisitedIsland(r, c, num_row, num_column,


条件式の真偽を反転して continue したほうが、ネストが浅くなって読みやすくなると思いました。

if (!isUnvisitedIsland(r, c, num_row, num_column, grid, is_visited_island)) { continue; } auto island_size = IslandSize(r, c, num_row,num_column, grid, is_visited_island); if (max_island_size < island_size) { max_island_size = island_size; }

ありがとうございます。
ご指摘の通り、先にcontinueでネストを深くしないべきだと気づきました

Create Max Area of Island.md

358573a

Fuminiton reviewed Apr 15, 2025

View reviewed changes

oda reviewed Apr 15, 2025

View reviewed changes

nodchip reviewed Apr 16, 2025

View reviewed changes

Conversation

irohafternoon commented Apr 14, 2025

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!

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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants