Conversation
| makeUpAmountToCoinCount[coinState.currentAmount()], | ||
| coinState.numCoins() | ||
| ); | ||
|
|
There was a problem hiding this comment.
ここの時点で更新がなされなかった場合、下の処理は不要ではないでしょうか。
たとえば、coins = {10, 11} だったとすると、1枚で作れるのは、10, 11 です。2枚で作れるのは、10 + 10, 10 + 11, 11 + 10, 11 + 11 がありますが、下の方の continue で、11 + 10 を弾いているつもりでしょうが、まだ 10 + 11 は Queue に入ったところなので、弾けていません。この後、3枚で作れる場合と考えていくと(コインの価格が十分に近いと Queue の中身は)、2^n で増えていきます。
There was a problem hiding this comment.
ご指摘ありがとうございます。仰る通りで、修正したら DFS の実装よりも早くなりました。
修正内容は step 4 に記載しました。
|
|
||
| ### 非再帰の DFS | ||
|
|
||
| 他の方の解法にあったので書いてみる。実行時間が上の解法より10倍以上長いが TLE にはならなかった。BFS の方が効率は良さそうに思えたが、テストケースとの相性がたまたまよかったのだろうか。 |
| * ※ n = amount, m = coins.length | ||
| */ | ||
| class Solution { | ||
| private final int CANNOT_BE_MADE_UP = -1; |
There was a problem hiding this comment.
private final int CANNOT_BE_MADE_UP = Integer.MAX_VALUE;とする。minCoinCountToMakeUpをすべて CANNOT_BE_MADE_UP で Arrays.fill() しておく。minCoinCountToMakeUp[0] = 0;としておく。
とすると、
for (int i = 1; i < minCoinCountToMakeUp.length; i++) {
for (int coin : coins) {
if (i - coin < 0 || minCoinCountToMakeUp[i - coin] == CANNOT_BE_MADE_UP) {
continue;
}
minCoinCountToMakeUp[i] = Math.min(minCoinCountToMakeUp[i], minCoinCountToMakeUp[i - coin]
}
だけとなり、すこしシンプルになると思います。最後、minCoinCountToMakeUp[amount] が CANNOT_BE_MADE_UP 出ないことを確認する手間は増えます。
| * 各インデックスの数字と同じ数値の額を何枚のコインで作れるかを保持する。 | ||
| * 計算の都合上、額が0の際は必要な枚数も0であるという情報が欲しいので、そのために要素数を + 1 している。 | ||
| */ | ||
| int[] minCoinCountToMakeUp = new int[amount + 1]; |
There was a problem hiding this comment.
自分なら amountToMinCoins と付けると思いますが、好みの問題かもしれません。
There was a problem hiding this comment.
充分意味が通じるのなら短いに越したことはないので、より良い選択肢に思えました。step 4 に書いた別解では上記の名前にしています: 3a99d93
| for (int coin : filteredCoins) { | ||
| int nextAmount = coinState.currentAmount() + coin; | ||
| if (nextAmount > amount) { | ||
| continue; |
There was a problem hiding this comment.
filteredCoinsが昇順に並んでいるので、一度nextAmountを超えたらそこでbreakしちゃって良いと思います。
| public int coinChange(int[] coins, int amount) { | ||
| Objects.requireNonNull(coins, "Argument coins must not be null"); | ||
|
|
||
| int[] filteredCoins = Arrays.stream(coins) |
There was a problem hiding this comment.
sortedのニュアンスも変数名にあっても良いかなとも思いました。あとはfilteredだけだと、どういう値が入っているかを想像しづらいかなと思いました。
| .sorted() | ||
| .toArray(); | ||
|
|
||
| int[] makeUpAmountToCoinCount = new int[amount + 1]; |
There was a problem hiding this comment.
個人的は変数名が長いわりにどういう値が入っているかを想像するのが難しかったです。まあ英語力の問題とかもあるかもですが。
|
|
||
| while (!coinStateStack.isEmpty()) { | ||
| CoinState coinState = coinStateStack.pop(); | ||
| makeUpAmountToCoinCount[coinState.currentAmount()] = Math.min( |
There was a problem hiding this comment.
この更新タイミングは、スタックに詰めるときで良いと思うんですよね。わざわざスタックから取り出した後に遅らせる必要性がないように感じます。L500で途中で抜ける最適化も早めに値を書き換えて上げたほうが効果があるように思います。
あとその場合L484のあとにmakeUpAmountToCoinCount[0] = 0をしてあげる必要がありますね。
4 participants
https://leetcode.com/problems/coin-change/description/