Conversation
| } | ||
|
|
||
| // 行が範囲外 | ||
| if adjacent.row < 0 || rowSize <= adjacent.row { |
There was a problem hiding this comment.
数直線をイメージしてその外側にあるからこの書き方なのでしょうか？
逆に!(0 <= row && row < rowSize)と書く方もいましたので紹介します。
| func exploreIsland(startCell Cell, grid [][]int) int { | ||
| rowSize := len(grid) | ||
| columnSize := len(grid[0]) | ||
| isValidLand := func(row, column int) bool { |
There was a problem hiding this comment.
ループの中で毎回これを生成するならば、外で関数定義したほうがいいと思います。
There was a problem hiding this comment.
そうですね、外に出すなら構造体のメソッドとして定義します。
| var maxArea int | ||
| for row := range grid { | ||
| for column := range grid[row] { | ||
| if grid[row][column] != LAND { |
There was a problem hiding this comment.
下にあるisValidLandを使う、あるいは不要な分岐を省きたければisValidとisLandにわかれていれば、ここも関数で同じように書けるように見えます。
There was a problem hiding this comment.
なお、今回のdfsの書き方は、陸であることを確認してから探検するという書き方ですが、海や境界外だったらすぐ帰るという書き方をすれば、関数最初にearly returnするだけで、exploreIslandを呼び出す際には海も境界も気にする必要がなくなるので、もう少し楽に書けると思います。
There was a problem hiding this comment.
そうですね、記述量は減ると思います。
私が読むとしたら、陸でないのに探索するのはなぜだろうと思ってしまうので、こんなふうに書いてしまいます。
| nextLands := []Cell{startCell} | ||
| for len(nextLands) > 0 { | ||
| land := nextLands[len(nextLands) - 1] | ||
| nextLands = nextLands[:len(nextLands) - 1] |
There was a problem hiding this comment.
ここに
grid[startCell.row][startCell.column] = WATER
を書けば、1個目の陸だけ特別扱いでこのfor文の前に沈めるとかせずに済んだように見えています。
There was a problem hiding this comment.
同じセルを2回以上スタックに積まないようにするために、入れる前に更新しています。
スタックから取り出した後に更新する場合、再度同じセルがスタックに積まれる可能性があると考えています。
スタックから取り出した後に更新する場合でも正しく大きさを計算できるか考えてみます。
| column int | ||
| } | ||
|
|
||
| func exploreIsland(startCell Cell, grid [][]int) int { |
There was a problem hiding this comment.
私の理解が足りていなかったのですが、この関数は引数のstartCellが陸かつvalidでないといけないんですよね。
せっかく再利用もできるのですから、関数の責務をどこまで持つかという話ですが、陸を要求するならばassertや例外スローなどでそれを明示しておいた方が関数のユーザーにとって使いやすいです。
| queue := []Cell{startCell} | ||
| for len(queue) > 0 { | ||
| cell := queue[0] | ||
| queue = queue[1:] |
There was a problem hiding this comment.
スライスを上書きしても、背後のデータはそのままなんですね（コピーされない）。Pythonだとコピーされてしまうので羨ましいです。
https://go.dev/blog/slices-intro
Slicing does not copy the slice’s data. It creates a new slice value that points to the original array.
| ### 1. 幅優先探索で島を探索し、面積の最大値を求める | ||
|
|
||
| - 以前やった島を数えるのとだいたい同じ | ||
| - grid をコピーする |
There was a problem hiding this comment.
訪問済みの島を管理するデータ構造について、選択肢の幅を検討できるとなお良いと思いました（前問で検討済みでしたらご放念ください）。
row, col->boolのMap[][]boolのスライス（gridと同じサイズ）
個人的には、処理中に WATER で塗りつぶしてしまうと、もとが島だったのかどうかの履歴が失われてしまい、デバッグにやや不便だったり、仕様変更や機能追加の際に手間がかかりそうに思いました。
There was a problem hiding this comment.
ありがとうございます。
別管理にする場合、状態を管理する構造体を定義し、そのメソッド内で参照する形にすると思います。
islandExplorer みたいな感じです
関数の引数として渡す形式にする場合、参照用の grid と管理用の何かを渡すので、それを嫌いました。
| column: land.column + offset.column, | ||
| } | ||
|
|
||
| if !isValidLand(adjacent.row, adjacent.column) { |
There was a problem hiding this comment.
valid は人によって想像する条件が異なりうるので、さらに具体的な命名にするとなお良いと思いました（今回の短さなら関数を読めばいいのですが、それすらスキップできる可能性もあります）。
- grid内である
LANDである- 訪問済みである
There was a problem hiding this comment.
ありがとうございます。
LANDをWATERに変える方針にしたので、フィットする関数名に悩みました。
いまだに思いついてはいないので、改善するとしたら下記から選ぶと思います。
- 関数ではなくそのまま記載する
- コメントでフォローする
|
全体的に読みやすかったです！ |
3 participants
今回の問題
Max Area of Island - LeetCode
使用言語
Go
次に解く問題
Maximum Depth of Binary Tree - LeetCode