695 Max Area of Island#17
Conversation
| int num_rows = grid.size(); | ||
| int num_cols = grid[0].size(); | ||
|
|
||
| DisjointUnionSet connected_component_manager(num_rows * num_cols); |
There was a problem hiding this comment.
manager という単語はよく見かけるのですが、この場合はどれくらい情報があるのが微妙に感じます。 connected_components はいかがでしょうか？
| // 本来ならconstをつけてコンストラクタで初期化したほうがよい | ||
| int num_rows; | ||
| int num_cols; | ||
| std::vector<std::vector<int>> unvisited_islands; |
There was a problem hiding this comment.
状態をメンバー変数として持たせた場合、複数のスレッドから呼び出したときに競合が起きたり、複数回呼び出したときに結果が意図しない結果になるといった問題が起こりえます。引数で引き回すことをおすすめします。
There was a problem hiding this comment.
たしかにSolutionクラスに複数スレッドからアクセスする場合引数に渡す必要がありますね…
本問題の場合最初にmaxAreaOfIslandが受け取るgrid自体は複数スレッドで共有しうると思うのですが、Solutionクラス自体は各スレッドで構築するシチュエーションが多いんじゃないかと思っていました。
複数スレッドをを想定する場合、メンバー変数にはconstなもののみ置いたほうがよいのでしょうか。
There was a problem hiding this comment.
複数スレッドを想定する場合、 const なメンバー変数は置いても安全なことが多いと思います。ただし、 const std::shared_ptr 等、その変数の先にオブジェクトがある場合は注意が必要です。そのオブジェクトを書き換える必要がある場合は、ロックを取ってから読み書きする、といった実装にする必要があると思います。
また、非 const なメンバー変数を置くこともあります。ただし、変更する関数が一つでもある場合は、ロックを取ってから読み書きをする、といった実装にすることになると思います。
ロックを取る際は、ロックを取りたい範囲や条件にもよりますが、 std::mutex や std::lock_guard などが使われると思います。
https://cpprefjp.github.io/reference/mutex/mutex.html
https://cpprefjp.github.io/reference/mutex/lock_guard.html
| int max_area_of_islands = 0; | ||
| // 1はまだ探索していない島、0は海または探索済みの島 | ||
| std::vector<std::vector<int>> unvisited_islands = grid; | ||
| for (const auto row : std::views::iota(0, num_rows)) { |
There was a problem hiding this comment.
自分は ranged-for 文でプリミティブ型には const を付けない派です。チームの平均的な書き方に合わせることをおすすめします。
| bool is_effective_grid(const int row, const int col) const { | ||
| return 0 <= row && row < num_rows && 0 <= col && col < num_cols; | ||
| } | ||
| // 島の連結成分の面積を計算し訪問済みにする |
| return 0; | ||
| } | ||
|
|
||
| unvisited_islands[row][col] = 0; |
There was a problem hiding this comment.
unvisited_islandsはvisited_islandsの方が個人的には分かりやすいと思いました。訪れた陸地(1)を水域(0)にマークしているよという感じだと思ったためです。
C++は雰囲気でしか読めない関係で文脈を読み取れておらず、おかしなことを言っていたらすみません。
There was a problem hiding this comment.
unvisited_islandsにすると、もともとの入力のgridをコピーしてそのまま陸地(1)、水域(0)とtrue/falseが一致するし、別にvisited_islandsを用意しなくていいので楽かなという意図でした。
たしかに、探索の動作とリンクしないとの、unvisited_islandsが0(false)を考えると二重否定になったりしてかえってややこしいかもしれません。。。
| private: | ||
| int num_rows; | ||
| int num_cols; | ||
| const std::array<std::pair<int, int>, 4> direction = {{{1, 0}, {0, 1}, {-1, 0}, {0, -1}}}; |
There was a problem hiding this comment.
所属するチームによって変わると思いますが、Googleのコーディング規約だと、定数名の頭にkを付けるらしいです。
https://google.github.io/styleguide/cppguide.html#Constant_Names
また、定数定義するときは、C++11以降だとconstexprが使えます。
4 participants
this problem
https://leetcode.com/problems/max-area-of-island/description/