Skip to content

Conversation

@TORUS0818
Copy link
Owner

思考ログ:
- DPで、STEP1からいくつか軽微な修正を加えた
- ループ変数```i```を```current_amount```へ
- 正直これもあんまりしっくり来ないが、```amount```が使われてしまっているのが痛い、、
Copy link

Choose a reason for hiding this comment

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

引数の変数名を変更するのは一つだと思いますね。
ただ、Keyword を使って呼ばれていると困ることになります。

プロダクションでもう使われているとして、誰が呼んでいるかを分からないコードを途中で変更するのは気が引けますが、Positional only にしておく手もあります。
https://docs.python.org/3/tutorial/controlflow.html#special-parameters

Copy link
Owner Author

Choose a reason for hiding this comment

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

Positional only知りませんでした。

return min_num_coins

return coin_change_helper(amount)
```
Copy link

Choose a reason for hiding this comment

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

1つの変数に2つ以上の情報を持たせる
coin_change_helper が num_coins, can_make を返すみたいな手もありますね。

-1 の扱いが気に入らなかったので Generator 使うのは考えてみました。

class Solution:
    def coinChange(self, coins: List[int], amount: int) -> int:
        @cache
        def coin_change_helper(amount):
            if amount == 0:
                return 0
            if amount < 0:
                return -1
            def possible_changes():
                for coin in coins:
                    num_coins = coin_change_helper(amount - coin)
                    if num_coins == -1:
                        continue
                    yield num_coins + 1
            return min(possible_changes(), default=-1)
        return coin_change_helper(amount)

Copy link
Owner Author

Choose a reason for hiding this comment

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

ありがとうございます。
Generatorいいですね!

そしてmin(default)の存在も頭から抜けてました。。

if amount == 0:
return 0

amount_and_num_coins = deque([(0, 0)])
Copy link

Choose a reason for hiding this comment

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

個人的にはキューの要素の中にコインの枚数を入れるのではなく、 list を 2 本用意して、コインが 1 枚増えるたびにリストを入れ替えていく書き方のほうが好きです。

if amount == 0:
    return 0
current_amounts = [0]
found = set()
num_coins = 1
while current_amounts:
    next_amounts = []
    for current_amount in current_amounts:
        for coin in coins:
            next_amount = current_amount + coin
            if amount == next_amount:
                return num_coins
            if amount < next_amount:
                continue
            if next_amount in found:
                continue
            found.add(next_amount)
            next_amounts.append(next_amount)
    current_amounts = next_amounts
    num_coins += 1
return -1

好みの問題かもしれません。

Copy link
Owner Author

Choose a reason for hiding this comment

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

ありがとうございます。
実は最近、リスト入れ替え方式のBFSを書くことが多いので、dequeの復習も兼ねての実装でした。

自分で書くと、
for num_coins, current_amount in enumerate(current_amounts, 1):
としてそうですが、num_coinsは頂いた実装のように別途足し上げていく方が分かりやすいなと思いました。

if not current_amount in amount_to_num_coins:
amount_to_num_coins[current_amount] = num_coins
else:
if amount_to_num_coins[current_amount] <= num_coins:
Copy link

Choose a reason for hiding this comment

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

DFS で書くと、コインの枚数の小さいほうから処理しないため、無駄な処理が起こるように思います。 BFS・DP で書くのが良いと思います。

if amount == 0:
return 0
if amount < 0:
return -1
Copy link

Choose a reason for hiding this comment

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

-1はどういう値かコメントが欲しいかもしれません

return next_num_coins

if next_amount > amount:
continue
Copy link

Choose a reason for hiding this comment

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

next_amountで突っ込む前にfoundをチェックする方法もありますね(多少速くなるかも?)

if not current_amount in amount_to_num_coins:
amount_to_num_coins[current_amount] = num_coins
else:
if amount_to_num_coins[current_amount] <= num_coins:
Copy link

Choose a reason for hiding this comment

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

個人的には>=の方がしっくりきますが好みの範囲かもしれません

class Solution:
def coinChange(self, coins: List[int], amount: int) -> int:
coins = sorted(coins)
amount_and_num_coins = deque([(0, 0)])
Copy link

Choose a reason for hiding this comment

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

dequeの機能は使ってないのでstack(単なるリスト)で良い気がします

Copy link
Owner Author

Choose a reason for hiding this comment

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

ありがとうございます。
list.pop(0)と勘違いしてました。

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