-
Notifications
You must be signed in to change notification settings - Fork 0
322. Coin Change #38
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
322. Coin Change #38
Conversation
| - 下記コードだと再ハッシュされるので、 | ||
| `changeableAmounts := make(map[int]struct{}, amount+1)` | ||
| とサイズを設定しても100msくらいかかった | ||
| - mapへの書き込みが思い? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
はい。思ったよりもハッシュ値の計算遅いんですよね。それに対して、配列の場合はポインター+足し算になり、CPU によっては、LEA など一命令でできるものもあります。
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LEAについて初耳だったので調べてみました。LEAはLoad Effective Addressの略でx86アーキテクチャのIntelやAMDにある命令でleal 8(%ebp) %eaxでebpアドレスに+8したアドレスがeaxアドレスに割り当てられる、という感じ。反対にARMなどのRISCアーキテクチャにはLEA命令がなく、加算とシフトを組み合わせて同等の計算をする
| - coins=[1,5,10], amount=9 -> 5 | ||
| - coins=[1,5,10], amount=16 -> 3 | ||
| - coins=[1,5,10], amount=25 -> 3 | ||
| - coins=[3,5,10], amount=7 -> -1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
価格が等しいコインがいくつかある場合のテストケースは欲しいかもしれません
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
すみません、問題文の要件を勘違いしてました
| - ループの中のif文が多くて美しくない | ||
| - ループの前後のエッジケース処理が美しくない | ||
| - 入出力の合致するコードになっただけ感 | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
0が三重の意味になっているのもややわかりにくそうですね
(amount = 0なら実際に0, まだ作れるか確かめていない場合、作ることができない場合)
|
|
||
| var coinChangeHelper func(accumulatedAmount int, coinCount int, sortedCoinsStartIndex int) | ||
| coinChangeHelper = func(accumulatedAmount, coinCount, sortedCoinsStartIndex int) { | ||
| minCoinCountForAmounts[accumulatedAmount] = min(minCoinCountForAmounts[accumulatedAmount], coinCount) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ここはminCoinCountForAmounts[accumulatedAmount] = coinCountでも大丈夫ですかね(L189-L190で弾いてるので)
minをあえてつけた方がわかりやすいという見方もあるかもしれませんが
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
本当ですね。ありがとうございます!
|
|
||
| - これがACコード | ||
| - coinsを降順にソートする際は参照透過性を保つためにcopyした | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
好みですが、自分は配列に保存して後でamountの時のコインの数探るよいうより、L184~のループの中でamountになった時点でその時点でのcoinの数を出力するコードにします。
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
これやると貪欲と一緒になっちゃいますね(何度もすみません)
https://leetcode.com/problems/coin-change/description/