Skip to content

Conversation

@fhiyo
Copy link
Owner

@fhiyo fhiyo commented Jun 11, 2024

- gridを書き換えるかは迷ったが、今回は許される環境だとして実装した。
- `m = len(grid)`, `n = len(grid[0])` のように長さを別途変数にするかも迷ったが、2次元配列のgridが長方形を表していることが分かりやすいかと思いこうした。
- gridをinner function内で触る際、nonlocalにしなくていいんだっけ?と気になったが、grid自体をrebindするわけではないので問題ない。
- WATERの変数を使ってないことは気になる。無いのもまた違和感あるので、コメントにしてもいいかもしれない

Choose a reason for hiding this comment

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

使っていないなら消してよいのではないかと思いました。
WATERが無いことの違和感は共感できませんでした。

Choose a reason for hiding this comment

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

趣味の問題だと思いますが、grid[i][j] is '0' or '1'.とあるのに、LAND = '1''1'だけが割り当てられているので、私は個人的になんか共感してしまいました。

Comment on lines +223 to +228
for row in range(height):
for col in range(width):
if grid[row][col] == Terrain.LAND.value:
for d_row, d_col in directions:
if inner_grid(row + d_row, col + d_col) and grid[row + d_row][col + d_col] == Terrain.LAND.value:
sections.union(row * width + col, (row + d_row) * width + (col + d_col))

Choose a reason for hiding this comment

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

ネストが深すぎると感じました。
自分なら、if文の条件を反転してcontinueを使って浅くすると思います。


class Node:
id_: int
terrain: Terrain

Choose a reason for hiding this comment

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

is_landとかでbool値にしてもいいかと思いました。
WATER使ってないみたいですし。
趣味の範囲かもしれません。

```py
from enum import Enum

class Terrain(Enum):

Choose a reason for hiding this comment

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

https://dictionary.cambridge.org/dictionary/english/terrain
an area of land, when considering its natural features

Copy link
Owner Author

Choose a reason for hiding this comment

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

良いクラス名思いつかなかったんですよね...

return list(filter(lambda n: n.terrain == Terrain.LAND and n == n.parent, self.nodes))

def union(self, x_id: int, y_id: int):
return self._link(self.findSet(x_id), self.findSet(y_id))

Choose a reason for hiding this comment

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

_linkの戻り値はvoidのようです。

- https://discord.com/channels/1084280443945353267/1183683738635346001/1194329732544737330
- https://discord.com/channels/1084280443945353267/1196472827457589338/1196541121912909824 の37

Union-Findでもできるらしいのでやってみた。全体的に出来が悪い気がするが、パッと改善案が思いつかない。

Choose a reason for hiding this comment

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

参考程度に私が書いてみたものです。

class UnionFind {
public:
    UnionFind(int n) : parent(n), rank(n) {
        iota(parent.begin(), parent.end(), 0);
    }

    int Find(int i) {
        if (parent[i] == i) {
            return i;
        }
        return parent[i] = Find(parent[i]);
    }

    bool Union(int i, int j) {
        i = Find(i);
        j = Find(j);
        if (i == j) {
            return false;
        }
        if (rank[i] > rank[j]) {
            swap(i, j);
        }
        parent[i] = j;
        if (rank[i] == rank[j]) {
            ++rank[j];
        }
        return true;
    }

private:
    vector<int> parent, rank;
};

class Solution {
public:
    int numIslands(vector<vector<char>>& grid) {
        const int nrows = grid.size(), ncols = grid[0].size();
        int result = nrows * ncols;
        UnionFind uf(nrows * ncols);
        auto index = [&](int r, int c) {
            return r * ncols + c;
        };
        for (int r = 0; r < nrows; ++r) {
            for (int c = 0; c < ncols; ++c) {
                if (grid[r][c] == '0') {
                    --result;
                    continue;
                }
                if (r + 1 < nrows && grid[r + 1][c] == '1' &&
                        uf.Union(index(r, c), index(r + 1, c))) {
                    --result;
                }
                if (c + 1 < ncols && grid[r][c + 1] == '1' &&
                        uf.Union(index(r, c), index(r, c + 1))) {
                    --result;
                }
            }
        }
        return result;
    }
};

Copy link
Owner Author

Choose a reason for hiding this comment

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

参考実装ありがとうございます。 03eed90 で書いてみました。

return 0 <= row < height and 0 <= col < width

sections = DisjointSetForest(grid)
directions = [(-1, 0), (1, 0), (0, -1), (0, 1)]

Choose a reason for hiding this comment

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

右と下だけで良さそうです


def merge(self, i: int, j: int) -> bool:
return self._link(
self.findSet(self.parent[i]),

Choose a reason for hiding this comment

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

ここでself.parent[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.

すみません、それはあまり意味ないと思います (一応、findSet()を呼び出す再帰の回数は減りそうですが)。

参考にしたアルゴリズムの教科書 (Introduction to Algorithmsの19.3 Disjoint-set forests) でそうやってると思いこんで書いたんですが、確認したら self._link(self.findSet(i), self.findSet(j)) で良さそうでした

self.parent = [i for i in range(size)]
self.rank = [0] * size

def findSet(self, i: int) -> int:
Copy link

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.

これは、path splittingとかpath halvingの話で合っていますか? (あと再帰をループに直すとか)
https://en.wikipedia.org/wiki/Disjoint-set_data_structure

Copy link

Choose a reason for hiding this comment

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

あ、そうです。

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.

5 participants