Skip to content

Conversation

@kazukiii
Copy link
Owner

問題へのリンク
https://leetcode.com/problems/max-area-of-island/description/

次に解く問題
323. Number of Connected Components in an Undirected Graph

README.mdへ頭の中の言語化と記録をしています。

def maxAreaOfIsland(self, grid: List[List[int]]) -> int:
LAND = 1
WATER = 0
n_row = len(grid)

Choose a reason for hiding this comment

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

rowの数を表しているかと思いますが、n_rowは省略しすぎな気がします。
num_rowsとかの方が個人的には良いと思いました。

Copy link
Owner Author

Choose a reason for hiding this comment

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

ありがとうございます。num_rows の方がいいですね。

visited = [[False] * n_col for _ in range(n_row)]

def calculate_area(row: int, col: int) -> int:
is_water = (
Copy link

@sakupan102 sakupan102 Jun 24, 2024

Choose a reason for hiding this comment

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

個人的にこれはrow, colを引数にとった関数として定義するほうが好みです。

Choose a reason for hiding this comment

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

is_water という名前に対して、領域内かの判定もしていることが気になりました。
役割をまとめるのであれば名前も is_valid_water_area のような形の方がいいのかなと思いました。
個人的には、area の種類が増えたりしても対応しやすいように、役割を分けたいと思いました。

Choose a reason for hiding this comment

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

関数の方がだいぶ見慣れている気はします。

Copy link
Owner Author

Choose a reason for hiding this comment

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

コメントありがとうございます。

@sakupan102
関数の方が好みなのはメインのロジックがすっきりするからでしょうか?

@goto-untrapped
You may assume all four edges of the grid are surrounded by water. とあるので、is_waterで十分な気がします。

Choose a reason for hiding this comment

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

関数が良いのは再利用ができると思ったからです。
今回は関係ないですが二つのエリアが水かかどうか同時に判定したい場合、is_water関数を定義しておけば同じようなことを書かなくても済みますよね。


visited[row][col] = True
area = 1
drow = (0, 1, 0, -1)

Choose a reason for hiding this comment

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

こちらもdはdeltaの略かと思いますが、省略せずにdelta_rowとする方が良いですかね

Copy link
Owner Author

Choose a reason for hiding this comment

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

こちらもdelta_rowの方がいいですね。ありがとうございます。

max_area = 0
for i in range(n_row):
for j in range(n_col):
if grid[i][j] == LAND and not visited[i][j]:

Choose a reason for hiding this comment

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

この条件に反しても0が返るのでなくてもよいかと思いました

Copy link
Owner Author

Choose a reason for hiding this comment

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

たしかにこの条件がなくても動きますが、それだと実装に依存したロジックになりませんか?

Choose a reason for hiding this comment

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

確かに関数の実装の仕方によって結果が変わる可能性はありますね。
ただ訪れたかどうかを関数の外と中で二重にチェックしているのも少し気になる気がします。
(ここら辺は怪しいですすみません...)

Comment on lines +20 to +21
drow = (0, 1, 0, -1)
dcol = (1, 0, -1, 0)

Choose a reason for hiding this comment

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

ペアとして使いたい場合、ペアで定義した方がいいのかなと思いました。
もっと量のあるコードの中にあった場合、対応していることが分かりやすいこととか、片方だけ代入していたら気付きやすいとか、思いました。

Copy link
Owner Author

Choose a reason for hiding this comment

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

ペアとして定義するのも良さそうです。ありがとうございます。

area = 1
drow = (0, 1, 0, -1)
dcol = (1, 0, -1, 0)
for direction in range(4):

Choose a reason for hiding this comment

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

drow や dcol を使うとか、定数で定義した方がいいのかなと思いました。

Copy link
Owner Author

Choose a reason for hiding this comment

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

おっしゃる通りです。マジックナンバーは避けた方がいいですね。

def calculate_area(row: int, col: int) -> int:
area = 0
visited[row][col] = True
node_que = deque([(row, col)])

Choose a reason for hiding this comment

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

名前がそのままな気がします。
いい名前が思いつきませんが、searching_areas とかでしょうか。。

Copy link
Owner Author

Choose a reason for hiding this comment

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

たしかにちょっと抽象的過ぎますね。いい名前が思いつきませんでした。
cells_to_visitにしようと思います。
sakupan102/arai60-practice#19 (comment)

area = 1
drow = (0, 1, 0, -1)
dcol = (1, 0, -1, 0)
for direction in range(4):
Copy link

Choose a reason for hiding this comment

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

この数字はdirectionを表すものでは無い気がします。iでいいかなと思いました。

Copy link
Owner Author

Choose a reason for hiding this comment

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

0-3は右下左上の4方向と一対一対応していると思いますが、微妙でしょうか、、?

Copy link

Choose a reason for hiding this comment

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

自分のdirectionのイメージは (0, 1) とか (-1, 0) とかで、それらとは違うかなと思ってコメントしました (for i in range(4) として (drow[i], dcol[i]) がdirectionという感じ)。
でも方向と1:1で対応した番号だからたしかにいいのかもしれない...すみません、ここのコメントは忘れてください 🙏

- Union Find
- 前回と同じ
- まずは実装しやすいDFSでやる
- 再帰の深さは最大でも250 -> 大丈夫
Copy link

Choose a reason for hiding this comment

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

m == grid.length
n == grid[i].length
1 <= m, n <= 50
なので最大50*50=2500じゃないでしょうか?

Copy link
Owner Author

Choose a reason for hiding this comment

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

2500ですね。ご指摘ありがとうございます。

def calculate_area(row: int, col: int) -> int:
area = 0
visited[row][col] = True
node_que = deque([(row, col)])
Copy link

Choose a reason for hiding this comment

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

queueのtypoか、省略でしょうか?あまり見ない書き方だとは思います。

Copy link
Owner Author

Choose a reason for hiding this comment

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

queはqueueの省略のつもりでした。ここの命名が微妙なので cells_to_visit に変更しようと思います。

and grid[next_row][next_col] == LAND
)
if is_land and not visited[next_row][next_col]:
visited[next_row][next_col] = True
Copy link

Choose a reason for hiding this comment

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

どちらが良いとかではないかもですが、自分はqueueに入れるのはこれから行く場所の予約で、popするときにvisitをする感覚なのでpop()の次の行にこの処理を書きますね (一応 visited[row][col] = True を1箇所書けば済むという利点もありそう)。

Copy link
Owner Author

Choose a reason for hiding this comment

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

その感覚には完全に同意なのですが、その書き方だとグリッドグラフの構造上、キューに重複したノードが入りませんか?

Copy link

Choose a reason for hiding this comment

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

たしかに...!自分が言ったような実装をする場合はpopした位置が訪問済みならcontinueするみたいな処理を入れないとダメですね。気づかなかったです、ありがとうございます 🙏

Comment on lines +37 to +38
drow = (0, 1, 0, -1)
dcol = (1, 0, -1, 0)
Copy link

Choose a reason for hiding this comment

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

drow = (0, 1)
dcol = (1, 0)

だけで良さそうです

Copy link
Owner Author

Choose a reason for hiding this comment

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

どのように使用するイメージでしょうか。右下左上の4方向に一対一対応させたい意図があります。

Copy link
Owner Author

Choose a reason for hiding this comment

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

あ、ここ理解しました!
この場合2方向のみで十分ですね。教えていただきありがとうございます。

@@ -0,0 +1,55 @@
class UnionFind:
Copy link

Choose a reason for hiding this comment

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

このUnionFindのアルゴリズム、自分にはなぜ動くのか分かりませんでした...
とくにparent_sizeが親のサイズ(?)を表しているように思えず。
たとえば [[1,1]] という入力に対して動かすと、parent_size[0]って-2になりませんか?

なにか参考にした資料などあるでしょうか?

Copy link
Owner Author

Choose a reason for hiding this comment

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

parent_size -> この変数名良くないですね。見直します。
一応この実装では、self.parent_size には親がいるときは親のノードを、親がいないときは根であることが判断できればよいのでマイナスに変換した木のサイズを入れてます。(バリエーションとしてrankをマイナスにして入れるような実装もたまに見ます)
以下のブログに詳細があります。
https://ikatakos.com/pot/programming_algorithm/data_structure/union_find_tree
https://note.nkmk.me/python-union-find/

可読性を考えるとシンプルにself.parentとself.sizeに分けた方が良さそうに思いました。

Copy link

Choose a reason for hiding this comment

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

self.parent_size には親がいるときは親のノードを、親がいないときは根であることが判断できればよいのでマイナスに変換した木のサイズを入れてます。

なるほど、理解できた気がします!

        self.parent_size[x] += self.parent_size[y]
        self.parent_size[y] = x

↑で上の行は常に負同士の足し算だから、マージ後の新しい根の側には負の値が入り、下の行で根じゃなくなった方には根の番号が入りますね。ありがとうございます 🙏

Copy link
Owner Author

Choose a reason for hiding this comment

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

はい、まさにその通りです。こちらこそコメントいただきありがとうございました!

visited = [[False] * n_col for _ in range(n_row)]

def calculate_area(row: int, col: int) -> int:
is_water = (
Copy link

Choose a reason for hiding this comment

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

私は、

if 範囲外:
  return 0
if :
  return 0
if visited[][]:
  return 0

と分けるかも知れません。

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.

6 participants