Join GitHub today
GitHub is home to over 50 million developers working together to host and review code, manage projects, and build software together.
Sign upGitHub is where the world builds software
Millions of developers and companies build, ship, and maintain their software on GitHub — the largest and most advanced development platform in the world.
Created greedy_coin_change.py file #3805
Conversation
added the file in greedy_methods folder to implement the same method
DevanshiPatel18
changed the title
|
Can someone review my PR? |
|
Here is my review. Thanks for contributing to open source! |
| """ | ||
|
|
||
|
|
||
| def find_minimum_change(V): |
mrmaxguns
Nov 5, 2020
Contributor
Variable names are lowercase, and one-letter variable names are old school. Also, type hints should be added:
def find_minimum_change(value: int) -> list:
...
Also, algorithms should not print values, they should return them. In this case, I suggest the function returns a list of integers, where each integer is a denomination.
Variable names are lowercase, and one-letter variable names are old school. Also, type hints should be added:
def find_minimum_change(value: int) -> list:
...Also, algorithms should not print values, they should return them. In this case, I suggest the function returns a list of integers, where each integer is a denomination.
| find_minimum_change(500) | ||
| 500 | ||
| find_minimum_change(0) | ||
| The total value cannot be zero or negetive |
mrmaxguns
Nov 5, 2020
Contributor
Change negetive -> negative. Codespell is pretty picky about misspellings, and the build will error.
Change negetive -> negative. Codespell is pretty picky about misspellings, and the build will error.
| def find_minimum_change(V): | ||
| total_value = int(V) | ||
| # All denominations of Indian Currency | ||
| denominations = [1, 2, 5, 10, 20, 50, 100, 500, 2000] |
mrmaxguns
Nov 5, 2020
Contributor
Maybe you could add an optional argument to the function so that the user can add their own custom denominations.
Maybe you could add an optional argument to the function so that the user can add their own custom denominations.
|
|
||
| # Traverse through all denomination | ||
| i = length - 1 | ||
| while i >= 0: |
mrmaxguns
Nov 5, 2020
Contributor
Use a for loop and loop through the denominations in reverse:
for denomination in reversed(denominations):
...
Use a for loop and loop through the denominations in reverse:
for denomination in reversed(denominations):
...| i -= 1 | ||
|
|
||
| # Print result | ||
| for i in range(len(answer)): |
mrmaxguns
Nov 5, 2020
Contributor
Don't print the result, return it:
return answer
Don't print the result, return it:
return answer
added the file in greedy_methods folder to implement the same method
Created the said file to implement the greedy coin change problem.
Checklist:
Fixes: #{$ISSUE_NO}.