Conversation
| LAND = 1 | ||
|
|
||
| def calc_area_size_from_unvisited(row, col, visited): | ||
| if row < 0 or num_rows <= row or col < 0 or num_cols <= col: |
There was a problem hiding this comment.
if not 0 <= row < num_rows or not 0 <= col < num_cols:など、数直線所に一直線上に並べたほうが、読み手にとって分かりやすくなると思います。
| num_rows = len(grid) | ||
| num_cols = len(grid[0]) | ||
|
|
||
| WATER = 0 |
There was a problem hiding this comment.
すみません、クラスインスタンスを調べたのですがあまりわかなくて、以下のどちらかを意味していますか?
この二つは自分も検討しましたがそれぞれ理由があって、採用しませんでした.
これは考慮すべきほど大きな問題じゃない(誤差)なのでしょうか。
class PLACE(Enum):
WATER = 0
LAND = 1アクセスしたい時に grid[row][col] == PLACE.WATER.valueのようになって.valueが挟まる分可読性が悪くなるかもしれないと考えたため不採用
class Solution:
WATER = 0
LAND = 1
def __init__(self):
....グローバルスコープにWATERとLANDが存在することになるため, バイトコードでLOAD_GLOBALが呼ばれる分, 関数内に定義した場合に呼ばれるLOAD_FASTよりもオーバーヘッドが大きいと考えたため不採用.
There was a problem hiding this comment.
- のほうを想定していました。
オーバーヘッドが大きいとのことですが、そのオーバーヘッドはどの程度 (例えば何 ms 程度) でしょうか?また、そのオーバーヘッドは、コードの読みやすさと比較して考慮すべきほどの大きさでしょうか?
なお、 Python で書いている時点で、 C/C++ などの高速な言語と比べると実行速度はけた違いに遅い場合が多いです。そのため、仮に LOAD_GLOBAL が LOAD_FAST より遅かったとしても、全体の実行時間に対する影響は相対的に小さいケースが多いように思います。
もしその程度の差でもパフォーマンスが問題になるのであれば、 Python 内で細かな最適化をするよりも、 C/C++/Rust/Java/C# など、より高速な言語で実装することを検討するほうが合理的な場面もあると思います。
There was a problem hiding this comment.
オーバーヘッドが大きいとのことですが、そのオーバーヘッドはどの程度 (例えば何 ms 程度) でしょうか?
10^4 * log10^4 stepで, 0.004秒 = 7msでした
1stepあたりで考えると, 0.004 * 10^3 * 10^3 / 10^4 = 0.4 ns 程度ですかね.
また、そのオーバーヘッドは、コードの読みやすさと比較して考慮すべきほどの大きさでしょうか?
コードの読みやすさが同じくらいならば考慮してもいいくらいだというのが自分の感覚です.
コードの読みやすさが著しく変わる(理解するのに+10secかかるくらい)ならば, 考慮しなくていい感覚です.
全体の実行時間に対する影響は相対的に小さいケースが多いように思います。
これはおっしゃる通りだと思っていて, その上でどのくらいのオーバヘッドから考慮すべきなのかの感覚が自分ではまだ自信がないです. ユースケースによると思いますが、 例えばリアルタイム性を重視するプログラムであれば, 同じコードの実行に0.1秒以上差があるなら, 相対的に小さいとは言えなくなってきそうな感覚があります.
もしその程度の差でもパフォーマンスが問題になるのであれば、 Python 内で細かな最適化をするよりも、 C/C++/Rust/Java/C# など、より高速な言語で実装することを検討するほうが合理的な場面もあると思います。
これはまさにその通りですね.
| areas += calc_area_size_from_unvisited(next_row, next_col, visited) | ||
| return areas | ||
|
|
||
| visited = [[False for _ in range(num_cols)] for _ in range(num_rows)] |
There was a problem hiding this comment.
visited [[False] * num_cols for _ in range(num_rows)]と書くと、少しシンプルになると思います。
|
|
||
| visited = [[False for _ in range(num_cols)] for _ in range(num_rows)] | ||
|
|
||
| max_areas = 0 |
There was a problem hiding this comment.
最大の面積は一つしかないため、単数形で max_area としたほうが違和感が少なくなると思います。
| if self.parent[idx] == idx: | ||
| return idx | ||
| parent_idx = self.find(self.parent[idx]) | ||
| self.parent[idx] = parent_idx | ||
| return parent_idx |
There was a problem hiding this comment.
'if self.parent[idx] != idx' の処理を先にすると return がまとめられそうです。
There was a problem hiding this comment.
def find(self, idx):
parent_idx = self.find(self.parent[idx])
self.parent[idx] = parent_idx
return parent_idxこういうことですかね. 確かにif文は不要になりそうです.
今回は、読み手のことを考えた時に(自然言語で動作を考えた時に), 「idxの親がidx自身だったらそのまま返す」ことを明示したかったという意図があります.
There was a problem hiding this comment.
あ、これ無限ループになりますね.
すみませんが、わからなかったので
'if self.parent[idx] != idx' の処理を先にすると return がまとめられそうです。
これどういうコードを想定しているか教えていただいてもいいですか?
[追記]
こういうことですかね.
アーリーリターンの方が読みやすそうだと思いますが好みの範囲ですかね
def find(self, idx):
if self.parent[idx] != idx:
parent_idx = self.find(self.parent[idx])
self.parent[idx] = parent_idx
return parent_idxThere was a problem hiding this comment.
以下のコードは、等号の場合に parent_idxが未定義となってしまいます
def find(self, idx):
if self.parent[idx] != idx:
parent_idx = self.find(self.parent[idx])
self.parent[idx] = parent_idx
return parent_idx
find は次のように書けます。
確かに意図を明確にすることも選択肢のうちの一つですね。僕の方はこっちの書き方に慣れているからか読みやすく感じます。自然言語にすると、自分の根が自分じゃないならば、自分の親を自分の根につけかえて、自分の親(根)を返す。という感じですね。
def find(self, idx):
if self.parent[idx] != idx:
self.parent[idx] = self.find(self.parent[idx])
return self.parent[idx]
There was a problem hiding this comment.
以下のコードは、等号の場合に parent_idxが未定義となってしまいます
そうでしたね... 気づいてなかったです
自然言語にすると、自分の根が自分じゃないならば、自分の親を自分の根につけかえて、自分の親(根)を返す。という感じですね。
確かに, こう考えると!=を使って最後にreturnまとめるのもわかりやすいですね。
ありがとうございます
| LAND = 1 | ||
| visited = [[False for _ in range(num_cols)] for _ in range(num_rows)] | ||
|
|
||
| def get_area_of_island(row, col): |
There was a problem hiding this comment.
DFS ですが、area_size を変数として持たずに、戻り値だけで表現する書き方もあります。移動した方向が有効ならば、 1 + 四方向の再帰結果 を返し、そうでないならば 0 を返すイメージです。そちらも確認しておいた方がよいのではないでしょうか。
There was a problem hiding this comment.
これですかね。確認しておきます。ありがとうございます
https://github.com/ksaito0629/leetcode_arai60/pull/17/changes#r2878146350
https://leetcode.com/problems/max-area-of-island/description/