-
Notifications
You must be signed in to change notification settings - Fork 0
200. Number of Islands.md #16
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
if (row, col) in seen or grid[row][col] == WATER: | ||
continue | ||
num_island += 1 | ||
traverse(row, col) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
traverse もう少し説明的な名前でもいいかもしれません。
関数は中を読まなくても、コードがまともそうであることが分かる名前が好まれます。
日本語でいうと「row, col を含む島を海に変える」くらいじゃないですか。
for delta_row, delta_col in [(0,1), (1,0), (0,-1), (-1,0)]: | ||
next_row = row + delta_row | ||
next_col = col + delta_col | ||
if 0 <= next_row < len(grid) and 0 <= next_col < len(grid[0]): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
このチェックは、pop の後に回すこともできますね。
if (row, col) in seen or grid[row][col] == WATER: | ||
continue | ||
seen.add((row, col)) | ||
for delta_row, delta_col in [(0,1), (1,0), (0,-1), (-1,0)]: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
個人的には、この四方向を一般化するよりは書き下したほうが読みやすいと思います。
代わりにここの中を関数にするか、簡略化するかして、
append(row, col - 1)
append(row, col + 1)
append(row - 1, col)
append(row + 1, col)
くらいの温度感で書けるといいと思います。
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
個人的には四方向を一般化する書き方のほうが好きです。現場においては、チームの平均的なメンバーが最短で正確に認知負荷低く読める書き方を選ぶのが良いと思います。
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ありがとうございます。常識と好みの範囲が見えた気がします!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
自分もこれくらいの処理なら書き下し派ですかね、8方向見るとか、それぞれに他の共通処理があるなどすれば一般化したくなるかなという感じです
islands = [(row, column)] | ||
while islands: | ||
island_row, island_column = islands.pop() | ||
if (island_row, island_column) in visited or grid[island_row][island_column] == '0': |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
このチェックを append() の直前に入れると、 append() と pop() の回数が減り、少し処理が軽くなると思います。ただ、好みの問題かもしれません。
if (row, col) in seen or grid[row][col] == WATER: | ||
continue | ||
seen.add((row, col)) | ||
for delta_row, delta_col in [(0,1), (1,0), (0,-1), (-1,0)]: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
個人的には四方向を一般化する書き方のほうが好きです。現場においては、チームの平均的なメンバーが最短で正確に認知負荷低く読める書き方を選ぶのが良いと思います。
class Solution: | ||
def numIslands(self, grid: List[List[str]]) -> int: | ||
WATER = '0' | ||
LAND = '1' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
この定数が未使用なので消してしまって良さそうですね。
if (row, column) in visited or grid[row][column] == '0': | ||
continue | ||
num_island += 1 | ||
islands = [(row, column)] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
island(島)は'1'(陸)の集合なのでlandsの方が正確だと思います
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ありがとうございます!
自分メモ:
islandの語源にlandが関係なくてびっくりした。
https://english-battle.com/word/island
```python | ||
class Solution: | ||
def numIslands(self, grid: List[List[str]]) -> int: | ||
num_island = 0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Go言語のgoogleスタイルガイドには変数名に型名を入れないようにとあり、num_islandよりisland_countのような変数名が好まれます。Pythonのスタイルガイドに似た記述がないか見てみましたが見つからなかったです。動的型付け言語では許容されるのかもしれません
参考: https://google.github.io/styleguide/go/decisions#variable-names
Omit types and type-like words from most variable names.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ご指摘ありがとうございます!
ご指摘の変数名に型名を入れないほうがいいという感覚は、Pythonでも共通のようです。
sakupan102/arai60-practice#13 (comment)
僕も同じ感覚でして、今回のnum_
はpythonでいうところのint型などの整数型のつもりではなく、numberという英単語のつもりでした。ですが、ご指摘の通り、個数を表すならcountの方がより表現できているなと気づきました。(また、numなら伝わるかもとは思いましたが、そもそも英単語の省略もイマイチでしたね。。。)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
変数名として正しくは、number_of_islands
でただこれだと長くてnum_islands
に略してもまあ伝わるかなという感覚です。まあ命名はチームのルールに合わせるのが良いと思うので、チームのみんながnum_
が分かりにくいということであれば、countもありだと思います
continue | ||
|
||
visited.add((island_row, island_column)) | ||
for dy, dx in [(0, 1), (1, 0), (0, -1), (-1, 0)]: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ここの順序が気になりました。1,2,3,4の全ての順序を書き出すときに、1234から始めて1243, 1324, 1342, ... のように接頭辞12を固定して、次は接頭辞を13に固定して、、、という風にすることによって抜け漏れがないように工夫すると思います。
なのでここも (0, 1), (0, -1), (1, 0), (-1, 0) の方が自然な順序なのではないかという気がしました。気にしすぎかもしれないです
https://leetcode.com/problems/number-of-islands/description/