min coin solution#118
Conversation
| arr.forEach((coin)=> { | ||
| let childCoins = 0; | ||
| if (value - coin > 0) { | ||
| childCoins = solution(arr, value - coin); |
| if (value - coin > 0) { | ||
| childCoins = solution(arr, value - coin); | ||
| } | ||
| if ( childCoins < currentMin) { |
There was a problem hiding this comment.
please try rewriting with Math.min function instead of the lines 42-44
| expect(solution([1, 2, 3, 6, 7], 12)).to.equal(2); | ||
| }); | ||
| it('simplest case [1], total=1', () => { | ||
| expect(solution2([1], 1)).to.equal(1); |
There was a problem hiding this comment.
Can you call your solution2 right behind solution in line 9? Refer to how it was done in this test file: https://github.com/llipio/algorithms/blob/master/test/13.js
dsope05
dismissed
yjlim5’s stale review
i don't know how to put MR into "comments fixed" state
| if (value - coin > 0) { | ||
| childCoins = solution2(arr, value - coin); | ||
| } | ||
| currentMin = Math.min(currentMin, childCoins); |
There was a problem hiding this comment.
Just one comment for more concise code in the future: If value - coin is not greater than 0, than we do not need to currentMin.
Other than that, the changes are approved.
|
@dsope05, that's ok. I get email notification for the changes, and your new commits are clearly shown in this thread. You should leave the "Changes Requested" state as is, so that I can "Approve Changes" after taking a look. :) |
2 participants
No description provided.