695. Max Area of Island

[TakayaShirai]

解いた問題：https://leetcode.com/problems/max-area-of-island/description/
次に解く問題：https://neetcode.io/problems/count-connected-components/question

[mamo3gr]

正常に動作して何らかの結果（ここでは0）を返すのか、例外を上げるのかの選択肢は検討しても良さそうです。
ちなみに、自分なら後者を選びます。というのも、自分がこの関数のユーザーであることを想像すると、グリッド（や行）が空であることの確認は呼び出し元でやりそうで、この分岐に引っかかるのはそのチェックをすり抜けてしまった場合なので、それなら例外で止めてもらったほうがバグに気が付きやすいからです。

[TakayaShirai]

レビューありがとうございます。

確かに 0 で返していると、デバッグ時に問題に気づくのが大変になりますね。参考になります！

[mamo3gr]

好みの範囲だと思いますが、calcAreaOfIsland のようなinner functionは別なメソッドとして括りだして、このメインループだけにしたいです。メソッドの行数が長くなって、コードを読むときに脳内のスタックに積んでおく情報が増えるのが、個人的には負荷が大きいです。何に使うかわからない関数が出てきた上でメインループが出てくるより、メインループが先に出てきて、じゃあサブルーチンで何をやってるのか見に行くか、という順番のほうがコードを読む心理的ハードルが下がる、というのもあります。
isLand くらいまでがinner functionにしても良い（個人的な）限度です。

[mamo3gr]

頭から読んだとき、以下のことが気になりました。

• key1の親のほうが小さいという保証があるのか？
  • →実際にはあとでサイズを見て入れ替える
• smaller == larger ってどういうことだ？サイズに大小あることを想定しているが、同じになる場合がある？
  • →この段階ではサイズは関係ない

大きさのことを気にするのは、親が違うことを確認してからでどうでしょうか。

Suggested change

	var smaller = find(key1);
	var larger = find(key2);
	if (smaller == larger) {
	return;
	}
	if (_sizes[smaller] > _sizes[larger]) {
	final tmp = smaller;
	smaller = larger;
	larger = tmp;
	}
	final parent1 = find(key1);
	final parent2 = find(key2);
	if (parent1 == parent2) {
	return;
	}
	final smaller = (_sizes[parent1] <= _sizes[parent2]) ? parent1 : parent2;
	final larger = (_sizes[parent1] <= _sizes[parent2]) ? parent2 : parent1;

（smaller, larger の代入の方法はこれがベストかは自信がありません。素直に var で入れてスワップの方がいいのかも）

[TakayaShirai]

ご指摘の通りだと思います。「親が同じであれば早期終了する」という文脈ではサイズは全く関係ないですね。ありがとうございます。

代入の方法は私も悩みます…。LLM にあげてもらったのだと以下のような候補がありそうです。

3 の var でスワップは、大きい方なのに smaller に入る矛盾するケースがあり、ここは割とデメリットなのかなと思っています。そのため、個人的にはそのデメリットがなく、一番シンプルな 1 がわかりやすく好みでした。ただ、二項演算子はあまり好みではにという方もいらっしゃるので、チームの好みに合わせるのが良さそうですね。

// 1. 二項演算子で一回でどちらにも代入
final (smaller, larger) = _sizes[parent1] <= _sizes[parent2] 
    ? (parent1, parent2) 
    : (parent2, parent1);

// 2. if で長めに明確に書く
late final int smaller;
late final int larger;

if (_sizes[parent1] <= _sizes[parent2]) {
  smaller = parent1;
  larger = parent2;
} else {
  smaller = parent2;
  larger = parent1;
}

// 3. var でスワップ
var smaller = parent1;
var larger = parent2;

if (_sizes[smaller] > _sizes[larger]) {
  (smaller, larger) = (larger, smaller) // この方式でも書けるの知らなかった。
}

// 4. sort を使う
final sorted = [parent1, parent2]
  ..sort((a, b) => _sizes[a].compareTo(_sizes[b]));

final smaller = sorted[0];
final larger = sorted[1];

[mamo3gr]

私は 1. でも違和感ありませんでした。文句を言うような人がいるなら 2. を選びます。

ところで late final なんて便利な機能もあるんですね。

[mamo3gr]

些末なところをコメントしましたが、全体的にとても読みやすかったです。

[sasanquaneuf]

インデントで2space/4spaceが混在していそうなのが少し気になりました。dartの一般的なルールを把握していないので、一般的に問題なければ大丈夫です。（ここのwhileが2spaceなのがずれているだけ？）

[TakayaShirai]

レビューありがとうございます。

最近 formatter を使うのを忘れており、意図せず混在していました。ご指摘ありがとうございます。混在は一般的ではないと思います。

[sasanquaneuf]

大きな問題ではないですが、後でループ変数のlandと名前が被っています。どちらかを別の名前にした方が良いかもしれません。

[sasanquaneuf]

ここの判定で!isLandを使っていない理由は何かありますか？

[TakayaShirai]

大した理由ではないのですが、isLand 関数は、ここの判定を書いた後に作成したため、isLand で置き換えられることに気づいていなかったという理由になります。コードを書いた順番としては、

1. 二重 for ループで、やりたいメインの処理を先に記載
2. calcAreaOfIsland の関数の中身を記述
3. calcAreaOfIsland の重複部分を isLand にまとめる

でした。