Skip to content

695. Max Area of Island#18

Open
seal-azarashi wants to merge 5 commits intomainfrom
max-area-of-island
Open

695. Max Area of Island#18
seal-azarashi wants to merge 5 commits intomainfrom
max-area-of-island

Conversation

@seal-azarashi
Copy link
Copy Markdown
Owner

int maxAreaOfIsland = 0;
int rowCount = grid.length;
int columnCount = grid[0].length;
boolean[][] isVisited = new boolean[rowCount][columnCount];
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

setを使って訪れたgridを管理する方法もありますね

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.

@nittoco
確かにそうですね。
Set に入れる値は "0-12" のような row, column, 値のセパレーターを含んだ string 値がいいかなと思ったのですが、いかがでしょうか?もしもっといい案があれば教えて頂けると嬉しいです。特になければ書いた通り実装してみようと思います。

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

すみません、返信を忘れていました🙇
Pythonだとタプルで実装したのですが、Javaだと似たのを作るのは大変と聞いたことがあるので(詳しくない)、それしかないかもですね、、、

continue;
}

areaOfIsland[0] = 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.

配列ではなく、int型の変数でもった方がいいかなと思いました。

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.

Java だと参照の値渡しをしてあげないと、引数として渡した先 (今回の場合 traverseIslands()) で値を書き換えることが出来ないので、苦し紛れにこう書いてました...

Copy link
Copy Markdown
Owner Author

@seal-azarashi seal-azarashi Sep 2, 2024

Choose a reason for hiding this comment

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

気持ち悪いと感じてはいるので、今では step 2 のように traverseIsland() が 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.

あれ、これ Boxed なやつを使うのが Java だったら標準的では。

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.

Boxed なやつを使う

int[] 型の代わりに Integer を使うということでしょうか?
Integer は参照型ではあるのですが immutable なので、この場合は意図した挙動にならないんですよね...

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

なんと。Java をあまり触っていないのがばれますね。あー、Map の key とかにしたいんですものね。

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

時間が大きく離れた中失礼します。

近い発想で、AtomicIntegerクラスをmutableなIntegerクラスとして使えるらしく、一応共有します。しかし配列と同様に本来の用途とずれており、すでに言及されているようにintを返り値に持つのが自分も素直かつベストだと感じます。

https://docs.oracle.com/javase/jp/21/docs/api/java.base/java/util/concurrent/atomic/AtomicInteger.html

https://stackoverflow.com/questions/23201468/is-it-good-practice-to-use-atomicinteger-as-a-substitute-for-mutable-integer

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

AtomicInteger はスレッドセーフで、そのためのオーバーヘッドが少しあります。ロックは取りませんが、他のスレッドがそのメモリーを使っていないことを確認しに行く必要があるので、10 ns くらいはかかりそうに思います。状況によっては許容でしょうが。ロック取るともう少し時間がかかります。
https://discord.com/channels/1084280443945353267/1300342682769686600/1357392579762716905
https://discord.com/channels/1084280443945353267/1300342682769686600/1357629082069893221

こういう数字はマシンによって変わるのでそんなに真面目に覚えるものではないです。

int を返したいときは return するか、自作クラスを作るかですね。Apache Commons Lang のようなユーティリティークラスにも Mutable があったりします。

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 [1] が使われることもあるようです。

時間との関係では、ライブラリーみたいなものはどのように使われるか分からないので、どのような用途でも高速にしておくモチベーションがあります。そうでないと状況次第ですが読みやすさを取る傾向が強いですね。

cells.push(new Cell(row, column));
while (!cells.isEmpty()) {
Cell cell = cells.pop();
boolean isInsideGrid = 0 <= cell.row && cell.row < grid.length && 0 <= cell.column && cell.column < grid[0].length;
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文にして、continueするのもありかと思いました。

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.

ありがとうございます。
ただこの判定、長い割にはそんなに重要なことをしてるわけでも無いのかなと個人的には思ってまして... 自分が読み手だったと考えた場合、 isInsideGrid みたいに何をしているのか一単語で表されていると嬉しいなと思い変数にしていました。

}

private int calculateAreaOfIsland(int[][] grid, boolean[][] isVisited, int row, int column) {
int areaOfIsland = 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.

細かいですが、変数名は何かしら整数を表すものにしたほうが良いかなと思いました。

areaOfIslandareaを表す変数に使われそうです

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.

すいません、整数を表す変数名というのが具体的にイメージ出来ていないのですが、例としてはどのようなものがありますでしょうか? numOfArea とかですかね👀

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

islandCount とか、numIslandとかですかね

Copy link
Copy Markdown
Owner Author

@seal-azarashi seal-azarashi Sep 3, 2024

Choose a reason for hiding this comment

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

なるほどです。変数名というよりは英語の話になりますが、 number は整数だけでなく浮動小数点数を示すイメージもあるので、整数を表すという目的で使うなら count が良さそうな気がします。

しかしちょっと難しいなと思うところもありまして、たとえば landCount(island あたりの land の数を求めるので land を使っています)とすると、なんだか遠回りで分かりづらくなってしまうような印象を抱きました。この問題においては Island に属する land の数とはつまり area(面積)なので、 area~ のような名前の方が直観的であるように個人的には思います。
面積の型が整数値というのは違和感があるのは同意なのですが、上記の理由で、個人的には landCount 等の遠回しな感じよりも、型の違和感の方がまだ許容できるかな、という感想でした。

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

あ、面積ってareaなんですか

すいませんこれをそもそもしらなかったもので🙏
areaOfIslandで大丈夫です

勉強になりました

// 空間計算量: O(m * n)
class Solution {
private static final int WATER = 0;
private record Cell(int row, int column) {};
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くらいのコードサイズだとあまりしなくてもどうにかなっちゃいますが、意図してクラスや型を作る練習もしようと思いました

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.

ありがとうございます!
Java は record が導入されてから上記のように簡潔に書けるようになったから良いのですが、これが無い以前の Java だったり、クラス定義にそれなりの記述量を要する言語だと、面接の時間内に書くのはちょっと大変かもしれないですね... ただ実務だと上記 Cell 相当のものを乱暴に要素数2の配列にするとかはさすがに厳しい印象なので、練習しておくのは大事だなと僕も思いました。

@Mike0121
Copy link
Copy Markdown

Mike0121 commented Sep 4, 2024

見ました。良いと思いました!

// 時間計算量: O(m * n)
// 空間計算量: O(m * n)
class Solution {
private int WATER = 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.

定数には private static final を付けるとよいと思いました。


public int maxAreaOfIsland(int[][] grid) {
int maxAreaOfIsland = 0;
int rowCount = grid.length;
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

自分は numRows といった名前を付けることが多いのですが、 rowCount でもまったく問題ないと思います。チームの平均的な書き方に合わせることをおすすめします。

int maxAreaOfIsland = 0;
int rowCount = grid.length;
int columnCount = grid[0].length;
boolean[][] isVisited = new boolean[rowCount][columnCount];
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

変数名が動詞の原型で始まると、関数名のように感じる場合があります。 visited で十分伝わると思います。

continue;
}

areaOfIsland[0] = 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.

自分なら traverseIslands() の返り値として面積を返すようにすると思います。

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.

8 participants