Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
261 changes: 261 additions & 0 deletions 695_max_area_of_island_medium/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,261 @@
# 問題へのリンク
[Max Area of Island - LeetCode](https://leetcode.com/problems/max-area-of-island)

# 言語
Python

# 問題の概要

与えられた2次元のグリッドにおいて、1は陸地、0は水を表す。最大の島の面積を求める問題です。島は上下左右に隣接する1の集合として定義される。

# 自分の解法
各マスを走査し、1が見つかったらそのマスを起点にグラフ探索を行い、島の面積を計算します。グラフ探索では連結成分を取り出せれば十分なので、走査の順番は特に気にしません。DFSでもBFSでも何でも解けますが、いずれにせよパフォーマンスを考えて、反復法で実装します。

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.

確かにその通りですね。そちらでもやってみます。



## step1

```python
class Solution:
def maxAreaOfIsland(self, grid: list[list[int]]) -> int:
# search for all cells. If island cell is found, then search for all connected island cells recursively. To reduce time complexity, we use visited cells.

WATER = 0
ISLAND = 1

height = len(grid)
if height<=0:
raise ValueError("grid height must be more than 0")
width = len(grid[0])

def is_valid(i:int,j:int)->bool:
return 0<=i<height and 0<=j<width


visited_cells:set[tuple[int,int]] = set()
max_area = 0
for i in range(height):
for j in range(width):
if (i,j) in visited_cells:
continue
cell = grid[i][j]
if cell == WATER:
continue

visited_cells.add((i,j))
area = 1

connected_islands = [(i,j)]
while connected_islands:

Choose a reason for hiding this comment

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

好みの範疇かもですが、ここはconnected_islandsの大きさが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.

レビューありがとうございます。
そうですね、グラフアルゴリズムではよく、(リストなりキューなりスタックなりが)空で無い限りループを回し、空になったらループを抜ける、と書く気がするので、そちらに合わせましたが、大きさ1以上と書いても良いのかもしれないですね。

i, j = connected_islands.pop()
for di, dj in [[0,1], [0,-1], [1,0], [-1,0]]:
i_neighbor, j_neighbor = i+di,j+dj
if not is_valid(i_neighbor, j_neighbor) or (i_neighbor, j_neighbor) in visited_cells:
continue

neighbor_cell = grid[i_neighbor][j_neighbor]
if neighbor_cell == ISLAND:
visited_cells.add((i_neighbor,j_neighbor))
connected_islands.append((i_neighbor, j_neighbor))
area += 1


max_area = max(area,max_area)
return max_area
```

- 時間計算量:`O(n*m)`
- 空間計算量:`O(n*m)`

## step2

```python
from collections import deque


class Solution:
def maxAreaOfIsland(self, grid: list[list[int]]) -> int:
"""
maxAreaOfIsland searches for all cells. If island cell is found and it is not visited yet, then search for all connected islands for it.
"""
if not grid or not grid[0]:
return 0

WATER = 0
ISLAND = 1
num_rows = len(grid)
num_columns = len(grid[0])

def is_valid(row: int, column: int) -> bool:
return 0 <= row < num_rows and 0 <= column < num_columns

def is_island(row: int, column: int) -> bool:
return is_valid(row, column) and grid[row][column] == ISLAND

def _calculate_area_bfs(
Copy link

Choose a reason for hiding this comment

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

関数名には実装の詳細や使用されているアルゴリズムより、その関数からどのような値が返ってくるかが書かれていて欲しいです。 _bfs は取り除いてよいと思います。

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.

inner function の関数名の先頭には _ は付けないことが多いと思います。他のモジュールやクラスから隠したい関数などは、先頭に _ を付けることが多いと思います。

https://google.github.io/styleguide/pyguide.html#3162-naming-conventions

Prepending a single underscore (_) has some support for protecting module variables and functions (linters will flag protected member access). Note that it is okay for unit tests to access protected constants from the modules under test.

initial_island: tuple[int, int], visited_cells: set
) -> int:
if not is_island(*initial_island):
return 0
area = 1
cells_to_visit = deque([initial_island])
DIRECTIONS = ((0, 1), (0, -1), (1, 0), (-1, 0))

while cells_to_visit:
row, column = cells_to_visit.popleft()
for dr, dc in DIRECTIONS:
neighbor = (row + dr, column + dc)
if is_island(*neighbor) and neighbor not in visited_cells:
Copy link

Choose a reason for hiding this comment

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

真偽を反転し、 continue し、ネストを下げたほうが読みやすくなると思います。

Copy link
Owner Author

Choose a reason for hiding this comment

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

以下のようにするということですよね。ありがとうございます。

if not is_island(*neighbor): continue
if neighbor in visited_cells: continue
...

このあたりは、いつも迷うところなのですが、何か具体的な方針などあるのでしょうか。今回のコードは↓のトレードオフがあるのかなと感じて、私は1を選択しました。

  1. 直接、条件を指定する方が、否定や余事象で指定するよりわかりやすい
  2. ネストは浅い方がわかりやすい

Copy link

Choose a reason for hiding this comment

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

タプルを関数の引数にアンパックして渡すのは、やろうとしていることに対して無用な複雑性を加えているように感じました。 step2_inplace などのように、 row_neighbor と column_neighbor に分解した状態で保持し、 visited_cells や cells_to_visit に入れる段階でタプルにしたほうが読みやすいと思います。

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_cells.add(neighbor)
cells_to_visit.append(neighbor)
area += 1
return area

visited_cells: set[tuple[int, int]] = set()
max_area = 0
for row in range(num_rows):
for column in range(num_columns):
cell = (row, column)
if is_island(*cell) and cell not in visited_cells:
visited_cells.add(cell)
area = _calculate_area_bfs(cell, visited_cells)
max_area = max(area, max_area)
return max_area
```

step1からの変更点
1. 責務の分離(リファクタリング):
* `step1`: 島の面積を計算するロジック(`while`ループ)が、グリッド全体を走査する`for`ループの中に直接ネストされていました。これにより、`maxAreaOfIsland`関数が長大で複雑になっていました。
* `step2`: 島の面積を計算するロジックを、独立したヘルパー関数 `_calculate_area_bfs` として切り出しました。これにより、`maxAreaOfIsland`関数は「グリッドを走査し、新しい島を見つけたら面積計算を依頼する」という単一の責務に集中でき、コードの可読性と保守性が向上しました。

2. 探索アルゴリズムの変更:
* `step1`: `list`をスタックとして利用し、`pop()`で要素を取り出す深さ優先探索(DFS)もどきを反復処理で実装していました。
* `step2`: `collections.deque`をキューとして利用し、`popleft()`で要素を取り出す幅優先探索(BFS)を実装しています。アルゴリズムとしては何でもいい(連結成分さえ取り出せればOK)ですが、BFSの方が有名で、読み手が理解しやすいと考えました。
Copy link

Choose a reason for hiding this comment

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

BFS と DFS は同じくらいの知名度かなと思いました。

Copy link
Owner Author

Choose a reason for hiding this comment

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

すみません、書き方が良くなかったです。ここでstackを用いるだけでは厳密にはDFSにならず、DFSもどきになるので、そうなるくらいなら典型的なBFSにした方が読み手がわかりやすいかなと考えた、の意です。


3. ヘルパー関数の追加:
* `step1`: 座標がグリッド範囲内かを確認する`is_valid`のみでした。
* `step2`: `is_valid`に加え、セルが陸地かどうかを判定する`is_island`関数を追加し、条件分岐の意図をより明確にしました。

4. エッジケースの処理:
* `step1`: `if height<=0:`で`ValueError`を送出していました。
* `step2`: `if not grid or not grid[0]:`という、よりPythonicで一般的な方法で空のグリッドをチェックし、`0`を返しています。
* LeetCodeの問題では制約条件が明確なので、わざわざ不要なエッジケースの処理を追加する必要はないかもしれませんが、例えばコーディング面接や実務の場では、どういう入力を想定するか、エッジケースはどう処理するかを話し合いながら決めていき、適宜エッジケースの処理を追加することが大切

5. コードの可読性向上:
* `step1`: `i`, `j`というループ変数が、外側の`for`ループと内側の`while`ループで再利用されており、混乱を招く可能性がありました。
* `step2`: 変数名を`row`, `column`のように、より意味の分かりやすいものに変更しました。また、方向を示すリストを`DIRECTIONS`という定数として定義し、マジックナンバーを排除しました。



## step3

- 基本的にはstep2と同じですが、タプルのアンパックなどを極力減らし、シンプルになるようにしました。

```python
from collections import deque


class Solution:
def maxAreaOfIsland(self, grid: list[list[int]]) -> int:
if not grid or not grid[0]:
return 0

num_rows = len(grid)
num_columns = len(grid[0])
ISLAND = 1

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.

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


def is_island(cell: tuple[int, int]) -> bool:

Choose a reason for hiding this comment

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

is_unvisited_land()として、visitedかどうかもこの中で確認してしまう手もあるかなと思いました。

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.

私は今回の状況はトレードオフにはなっていないかな、と感じでいるので、私の理解を記載してみます。

  • 課題感:
    • 該当箇所: if is_island(*neighbor) and neighbor not in visited_cells
      • セットで2回繰り返し使われているのでまとめても良さそう
      • 2つの条件を同時に考えているので、前の条件の結果を覚えたまま次のことを考える必要があり、脳のワーキングメモリを少し多めに使用する
  • 提案した対応案
    • is_island と not in visited_cells をまとめて条件判定できる、is_unvisited_land()を用意することで、条件判定を一つにしてしまう
  • 感じ方の齟齬がありそうに思っている部分
    • is_unvisited_land()という明確な名前のついた関数ならば、中身の詳細を覚えておく必要はないのでワーキングメモリは使わない VS 複雑な関数を作ると中身を覚えておく必要がありワーキングメモリを使う

どういうものを読むと脳に負担を感じるかなど、個人差があるかもしれないので、あくまでそういう人もいるんだなと受け取っていただければと思います。

Copy link
Owner Author

Choose a reason for hiding this comment

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

丁寧にありがとうございます。(返信遅れてしまってごめんなさい)
そうですね。たしかにそこまできちんと考えてみると、is_unvisited_island 関数を入れてしまった方がわかりやすいと感じるようになりました。ありがとうございます。

Choose a reason for hiding this comment

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

cell よりは position の方が読みやすく感じるのですが、好みだと思います。そもそもでいうと、私なら row, col を引数にするかなと思います。

row, column = cell
return (
0 <= row < num_rows
and 0 <= column < num_columns
and grid[row][column] == ISLAND
)

def calculate_area_bfs(
initial_island: tuple[int, int], visited_cells: set

Choose a reason for hiding this comment

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

initial_"island" が座標 (cell) を表すというのが不一致であるように感じます。下記コメントのように frontier / explored にするか、initial_row, initial_col、もしくは start_position のような命名にすると思います。

Copy link
Owner Author

Choose a reason for hiding this comment

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

確かにcell, row/columnに加えてinitial_islandも使うと混乱を生みかねないですね。修正した方がわかりやすそうだなと感じました。

) -> int:
if not is_island(initial_island):
return 0

cells_to_visit = deque([initial_island])
Copy link

Choose a reason for hiding this comment

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

自分なら frontier と explored と名付けるのですが、趣味の範囲だと思います。

visited_cells.add(initial_island)
area = 1
DIRECTIONS = ((0, 1), (0, -1), (-1, 0), (1, 0))
while cells_to_visit:
row, column = cells_to_visit.popleft()
for dr, dc in DIRECTIONS:
neighbor = (row + dr, column + dc)
if is_island(neighbor) and neighbor not in visited_cells:
visited_cells.add(neighbor)
cells_to_visit.append(neighbor)
Comment on lines +188 to +190

Choose a reason for hiding this comment

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

一旦全部 queue に入れて、pop したときに判定するという手もありますね。個人的にはその方がきれいに書けるような気がします。

area += 1
return area

max_area = 0
visited_cells = set()
for row in range(num_rows):
for column in range(num_columns):
cell = (row, column)
if is_island(cell) and cell not in visited_cells:
area = calculate_area_bfs(cell, visited_cells)
max_area = max(max_area, area)

Choose a reason for hiding this comment

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

個人的には199行目あたりは以下のように書くのもいいと思っています。
if文として覚えておくべき条件を減らしたいからです。
また、ループでやりたいことはareaの最大サイズの更新なので、「ある条件の時だけ更新する」というより「ある条件の時は更新を行わない」の方が個人的には自然な気がしています。

                cell = (row, column)
                if not is_island(cell):
                    continue
                if cell in visited_cells:
                    continue
                area = calculate_area_bfs(cell, visited_cells)
                max_area = max(max_area, area)

ここらへんの話が近い話だと思っています。
https://discord.com/channels/1084280443945353267/1200089668901937312/1207200647594639391

Copy link
Owner Author

Choose a reason for hiding this comment

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

レビューありがとうございます。
皆さんのコメントも見ながら、こちらの方がコードを読んでいるときのワーキングメモリーの利用が確かに節約できるなという感覚がわかってきました。early returnなども含めて意識していきます。

return max_area
```

## step4 (FB)

- 座標はtupleでまとめてしまうより、`row`, `column`で分けた方が読みやすいという意見をいただきました。確かに、二つ、三つ程度の座標を扱う場合は、分けた方が可読性が上がると感じました。
- `initial_island`はわかりにくい、ともコメントいただきました
- DFS/BFSのアルゴリズムで使う`DIRECTION=...`の部分は先にすべてqueueにpushして、popしたときに判定する方法もある。
-
-
### if文の条件分岐について
```python
while X:
if A and B:
Y
```
において、Yの処理がある程度長かったら

```python
while X:
if not A:
continue

if not B:
continue
Y
```
と書き換えた方がいい。今回なら

```python
for row in range(num_rows):
for column in range(num_columns):
cell = (row, column)
if is_island(cell) and cell not in visited_cells:
area = calculate_area_bfs(cell, visited_cells)
max_area = max(max_area, area)
```

```python
for row in range(num_rows):
for column in range(num_columns):
cell = (row, column)
if not is_island(cell):
continue
if cell in visited_cells:
continue
area = calculate_area_bfs(cell, visited_cells)
max_area = max(max_area, area)
```
と書き換えるとわかりやすい(とのコメントをいただいたし、共感している)。

# 別解・模範解答
`grid`に対してinplaceな実装をして`visited_cells`を管理すれば空間計算量は`O(1)`になりますが、それは問題の要件依存だと思いました。`step2_inplace.py`ではそのような実装をしています。

- 時間計算量:`O(n*m)`
- 空間計算量:`O(1)`

# 次に解く問題の予告
-
59 changes: 59 additions & 0 deletions 695_max_area_of_island_medium/step1.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
#
# @lc app=leetcode id=695 lang=python3
#
# [695] Max Area of Island
#

# @lc code=start


class Solution:
def maxAreaOfIsland(self, grid: list[list[int]]) -> int:
# search for all cells. If island cell is found, then search for all connected island cells recursively. To reduce time complexity, we use visited cells.

WATER = 0
ISLAND = 1

height = len(grid)
if height <= 0:
raise ValueError("grid height must be more than 0")
width = len(grid[0])

def is_valid(i: int, j: int) -> bool:
return 0 <= i < height and 0 <= j < width

visited_cells: set[tuple[int, int]] = set()
max_area = 0
for i in range(height):
for j in range(width):
if (i, j) in visited_cells:
continue
cell = grid[i][j]
if cell == WATER:
continue

visited_cells.add((i, j))
area = 1

connected_islands = [(i, j)]
while connected_islands:
i, j = connected_islands.pop()
for di, dj in [[0, 1], [0, -1], [1, 0], [-1, 0]]:
i_neighbor, j_neighbor = i + di, j + dj
if (
not is_valid(i_neighbor, j_neighbor)
or (i_neighbor, j_neighbor) in visited_cells
):
continue

neighbor_cell = grid[i_neighbor][j_neighbor]
if neighbor_cell == ISLAND:
visited_cells.add((i_neighbor, j_neighbor))
connected_islands.append((i_neighbor, j_neighbor))
area += 1

max_area = max(area, max_area)
return max_area


# @lc code=end
62 changes: 62 additions & 0 deletions 695_max_area_of_island_medium/step2.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
#
# @lc app=leetcode id=695 lang=python3
#
# [695] Max Area of Island
#

# @lc code=start

from collections import deque


class Solution:
def maxAreaOfIsland(self, grid: list[list[int]]) -> int:
"""
maxAreaOfIsland searches for all cells. If island cell is found and it is not visited yet, then search for all connected islands for it.
"""
if not grid or not grid[0]:
return 0

WATER = 0
ISLAND = 1
num_rows = len(grid)
num_columns = len(grid[0])

def is_valid(row: int, column: int) -> bool:
return 0 <= row < num_rows and 0 <= column < num_columns

def is_island(row: int, column: int) -> bool:
return is_valid(row, column) and grid[row][column] == ISLAND

def calculate_area_bfs(
initial_island: tuple[int, int], visited_cells: set
) -> int:
if not is_island(*initial_island):
return 0
area = 1
cells_to_visit = deque([initial_island])
DIRECTIONS = ((0, 1), (0, -1), (1, 0), (-1, 0))

while cells_to_visit:
row, column = cells_to_visit.popleft()
for dr, dc in DIRECTIONS:
neighbor = (row + dr, column + dc)
if is_island(*neighbor) and neighbor not in visited_cells:
visited_cells.add(neighbor)
cells_to_visit.append(neighbor)
area += 1
return area

visited_cells: set[tuple[int, int]] = set()
max_area = 0
for row in range(num_rows):
for column in range(num_columns):
cell = (row, column)
if is_island(*cell) and cell not in visited_cells:
visited_cells.add(cell)
area = calculate_area_bfs(cell, visited_cells)
max_area = max(area, max_area)
return max_area


# @lc code=end
Loading