63. Unique Paths II

[seal-azarashi]

https://leetcode.com/problems/unique-paths-ii/description/

[fhiyo]

キャッシュはそのkeyに対応する値が存在すれば値を算出するための計算をスキップしてその値を返すものとして使われるイメージがあり、この変数にキャッシュと名付けるのは自分には少し違和感がありました。
https://en.wikipedia.org/wiki/Cache_(computing)

[seal-azarashi]

@fhiyo
返信が大変遅くなりました。

この変数は命名するのに悩みました。確かに各イテレーションでは計算のスキップは行われないので、cache が名前に含まれている違和感があるのは理解できます。
少し考えてみたのですが、uniquePathCounts とするのはいかがでしょうか？これでしたら key に対応する値がユニークな path の数である以外の意味は含まれないので、上記のような違和感はないかと思います。

[Yoshiki-Iwasa]

変数を左辺に持ってくるのが一般的と思います

[oda]

私は、0 < x のほうがマジックナンバー感がなくて好きですね。趣味の範囲でしょう。

[Yoshiki-Iwasa]

1 がマジックナンバーになってるので、名前をつけてあげてほしいです

[seal-azarashi]

@Yoshiki-Iwasa
返信が大変遅くなりました。

仰るとおりですね。step 2 以降定数として宣言するようにしました。

[Yoshiki-Iwasa]

このuniquePathCacheという配列についてコメントで説明がほしいです

uniquePathCache[i] が ある列におけるi行目に到達する方法の数だというのは名前から自明でなく、コードを読んでいかないとわからないので

[seal-azarashi]

@Yoshiki-Iwasa
返信が大変遅くなりました。

そうですね、確かにコメントがあった方が良さそうです。「スタート地点から [i] に到達するまでのステップ数をキャッシュ」と書こうかと思いましたが、充分な説明になってますでしょうか？

[nittoco]

ここで、

if(this.obstacleGrid[y][x] == 1){
            return;
        }

とすれば、L27~29と、L45, L49の後半の条件がいらなくなりますね。

[seal-azarashi]

@nittoco
返信が大変遅くなりました。

たしかにそうですね。Step 4 に修正版を加えました: 83d0360

[nittoco]

個人的にここのコードは読みにくかったです。
このロジックで実装するなら、L116~L131までのどこかで関数化した方が読みやすいかもしれません。

[seal-azarashi]

@nittoco
返信が大変遅くなりました。

読みづらさの原因は関数が長いことでしょうか？個人的には30行程度なのであまり違和感を感じませんでしたが、例えば次のようにすれば読みやすくなるでしょうか。

class Solution {
    public int uniquePathsWithObstacles(int[][] obstacleGrid) {
        int rowCount = obstacleGrid.length;
        int columnCount = obstacleGrid[0].length;
        int[] uniquePathCache = createUniquePathCache(columnCount, obstacleGrid);
        int leftColumnFirstObstacleIndex = createLeftColumnFirstObstacleIndex(rowCount, obstacleGrid);

        for (int y = 1; y < rowCount; y++) {
            if (leftColumnFirstObstacleIndex <= y) {
                uniquePathCache[0] = 0;
            } else {
                uniquePathCache[0] = 1;
            }
            for (int x = 1; x < columnCount; x++) {
                if (obstacleGrid[y][x] == 1) {
                    uniquePathCache[x] = 0;
                    continue;
                }
                uniquePathCache[x] += uniquePathCache[x - 1];
            }
        }
        return uniquePathCache[columnCount - 1];
    }

    private int[] createUniquePathCache(int columnCount, int[][] obstacleGrid) {
        int[] uniquePathCache = new int[columnCount];
        for (int i = 0; i < columnCount; i++) {
            if (obstacleGrid[0][i] == 1) {
                break;
            }
            uniquePathCache[i] = 1;
        }
        return uniquePathCache;
    }

    private int createLeftColumnFirstObstacleIndex(int rowCount, int[][] obstacleGrid) {
        int leftColumnFirstObstacleIndex = rowCount;
        for (int i = 0; i < rowCount; i++) {
            if (obstacleGrid[i][0] == 1) {
                leftColumnFirstObstacleIndex = i;
                break;
            }
        }
        return leftColumnFirstObstacleIndex;
    }
}

関数化をしたらしたで、引数に何が来るのかとかを把握しながら読むことになるので、意外とひとつにまとまっていた方が読みやすかったりしないかなとも思ったりするのですが、どうでしょうね。

[nittoco]

細かくて趣味の範囲そうですが、

uniquePathCache[0] = 1;
if (leftColumnFirstObstacleIndex <= y) {
                uniquePathCache[0] = 0;
} 

と自分ならするかも(障害があるより向こう側は0になるというのが特別な感じがするので)

[seal-azarashi]

なるほど、これも良いですね。else 句は適切に使えてるのかいつも悩ましいです。