Skip to content

Comments

DP 1 done#1984

Open
pranjay01 wants to merge 1 commit intosuper30admin:masterfrom
pranjay01:master
Open

DP 1 done#1984
pranjay01 wants to merge 1 commit intosuper30admin:masterfrom
pranjay01:master

Conversation

@pranjay01
Copy link

No description provided.

@super30admin
Copy link
Owner

Strengths:

  • You have attempted to use dynamic programming for the Coin Change problem, which is the right approach.
  • You recognized the need to initialize a DP array and iterate over coins and amounts.
  • Your code for the House Robber problem is efficient and uses constant space.

Areas for Improvement for Coin Change:

  • The DP array should be initialized once outside the coin loop. Currently, you are resetting prevArr[0] = 0 for every coin, which is unnecessary. The base case (amount=0 requires 0 coins) should be set once at the beginning.
  • When updating prevArr[i], you should check if prevArr[i-coin] is not infinity before adding 1. Otherwise, adding 1 to infinity will cause issues (though in Python, 1 + math.inf is still infinity, which is acceptable, but it's good practice to avoid such operations).
  • The typical bottom-up approach for coin change initializes dp[0] = 0 and dp[i] = float('inf') for i>0. Then for each coin, for each amount from coin to the total amount, update dp[i] = min(dp[i], 1 + dp[i-coin]).
  • Your current code does not correctly accumulate the minimum coins across all coins because you are updating the same array without storing the previous state for other coins. Actually, your inner loop goes from 0 to amount, which is correct for an unbounded knapsack, but the update should be: dp[i] = min(dp[i], 1 + dp[i-coin]) only if i-coin >=0. However, you are using a single array and updating it as you go, which is correct for the unbounded case. But the error is in resetting the base case repeatedly and not initializing properly.

Corrected code for Coin Change:

def coinChange(self, coins: List[int], amount: int) -> int:
    dp = [float('inf')] * (amount+1)
    dp[0] = 0
    for coin in coins:
        for i in range(coin, amount+1):
            dp[i] = min(dp[i], 1 + dp[i-coin])
    return dp[amount] if dp[amount] != float('inf') else -1

Note: This corrected code iterates for each coin and for each amount from the coin value to the total amount. This avoids the need to check if the coin is greater than the amount in the inner loop.

  • Also, avoid using math.inf if you can use float('inf') for consistency, but both are acceptable.

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.

2 participants