Skip to content
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

Adding approach change #2859

Merged
merged 30 commits into from
Nov 25, 2024
Merged

Adding approach change #2859

merged 30 commits into from
Nov 25, 2024

Conversation

jagdish-15
Copy link
Contributor

pull request

This pull request adds an approach for the "Change" exercise in the Java track, based on discussions with @kahgoh during mentoring.

I was unsure how to generate a uuid for the approach, so I've left it empty for now. I would greatly appreciate guidance on how to generate one.


Reviewer Resources:

Track Policies

@jagdish-15
Copy link
Contributor Author

There's only one issue causing the unsuccessful check i.e. the uuid.

Copy link
Member

@kahgoh kahgoh left a comment

Choose a reason for hiding this comment

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

Sorry it took me a while to get to this one @jagdish-15. Looking at the content, it looks the structure and formatting differ quite a bit from the existing ones, so the comments are mostly trying to become more consistent with the existing ones.

@@ -0,0 +1,86 @@
# Dynamic Programming Approach

The **Dynamic Programming (DP)** approach is an efficient way to solve the problem of making change for a given total using a list of available coin denominations. It minimizes the number of coins needed by breaking down the problem into smaller subproblems and solving them progressively.
Copy link
Member

Choose a reason for hiding this comment

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

Looking at how we have written the content for some of other approaches (like boolean chain for leap, bit shifting for grains and if statements for Bob, I'd suggest moving the code up here, just under the heading. The description of the approach then begins just under it.

Also, we generally use one sentence per line in our Markdown content. For example:

The **Dynamic Programming (DP)** approach is an efficient way to solve the problem of making change for a given total using a list of available coin denominations.
It minimizes the number of coins needed by breaking down the problem into smaller subproblems and solving them progressively.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure, I will move the code to the top and start the explanation afterward. I have also used new lines for every sentence separately, except when it is in a bullet point.


- **Efficiency**: This approach is highly efficient in terms of minimizing the number of coins, but it might require significant memory for larger `grandTotal` values, as the space complexity grows linearly with `grandTotal`.

- **Alternative Approaches**:
Copy link
Member

Choose a reason for hiding this comment

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

Could you please remove this section on alternative approaches? This should just focus on the dynamic programming approach as this will become the dynamic programming page. Alternatively, we could move it to a Which approach to use? section in the introduction.md (similar to the isogram or robot name approaches), but we probably should briefly describe the Greedy Approach if we do (and we could possibly move the point about the efficiency there too).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have removed both the "Efficiency" and "Alternative Approach" sections entirely. I agree that it makes more sense to include these points in the introduction file under a section like "Which Approach to Use?" However, I believe this should be done only after we add another approach to compare. For now, with just a single approach, these sections are unnecessary.

@@ -0,0 +1,26 @@
# Introduction to Change Calculator
Copy link
Member

Choose a reason for hiding this comment

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

This suggestion is to make the heading consistent with our other existing approaches.

Suggested change
# Introduction to Change Calculator
# Introduction

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Indeed!!


In the "Change Calculator" exercise, the goal is to determine the minimum number of coins needed to reach a given total using a specific set of coin denominations. This is a classic problem in dynamic programming, where efficient change-making is essential, especially when there are constraints on coin types or large totals.

## Problem Overview
Copy link
Member

Choose a reason for hiding this comment

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

I don't think we need to repeat the exercise here. The other approaches generally have a "General guidance" section that talks about the "key things" to solve the exercise - things that all approaches are likely to have to. For example, in leap, it is as simple as knowing whether a year is divisible by 400, 100 and 4. For grains, its knowing that each square is doubled.

For Change, it could be that not all totals can be reached and somehow figuring out which ones can be reached and how.

Suggested change
## Problem Overview
## General guidance

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure, I've changed it.


The solution should find the optimal combination of coins to match the total. If it's impossible to match the total exactly, the solution should indicate this by throwing an exception.

## Approach Overview
Copy link
Member

Choose a reason for hiding this comment

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

The other approaches generally have a short listing of all the approaches here, with a link to the specific approach's page just underneath. For example, this would normally be a code listing for your dynamic programming approach followed by a link to the page generated from dynamic-programming/content.md (the URL would be https://exercism.org/tracks/java/exercises/change/approaches/dynamic-programming).

Suggested change
## Approach Overview
## Approach: Dynamic programming

- **Flexibility**: Handles cases where exact change is impossible by checking at each step.
- **Scalability**: Works for various coin denominations and totals, though large inputs may impact performance.

For a detailed look at the code and logic, see the full explanation in the [approach file](https://exercism.org/tracks/java/exercises/change/approaches/dynamic-programming).
Copy link
Member

Choose a reason for hiding this comment

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

In regards to the approach file link, there is a preference to use reference linking to make maintenance easier.

I also suggest changing the link text to "Dynamic Programming approach" as students will see a web page based on your content.md (and not the content.md "file").

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree, and I've made the changes accordingly.

"jagdish-15"
],
"contributors": [
"kagoh"
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
"kagoh"
"kahgoh"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am sorry for the oversight.

@jagdish-15
Copy link
Contributor Author

Thank you so much, @kahgoh, for all these suggestions. Please let me know if there's anything else I should improve, change, or remove.

Copy link
Member

@kahgoh kahgoh left a comment

Choose a reason for hiding this comment

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

Thanks @jagdish-15, that looks better!

The **Dynamic Programming (DP)** approach is an efficient way to solve the problem of making change for a given total using a list of available coin denominations.
It minimizes the number of coins needed by breaking down the problem into smaller subproblems and solving them progressively.

This approach ensures that we find the most efficient way to make change and handles edge cases where no solution exists.
Copy link
Member

Choose a reason for hiding this comment

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

I find this sentence redundant because all the approaches will need to do this to successfully solve the exercise.

Suggested change
This approach ensures that we find the most efficient way to make change and handles edge cases where no solution exists.

Copy link
Contributor Author

@jagdish-15 jagdish-15 Nov 18, 2024

Choose a reason for hiding this comment

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

Yep, I can't deny! I will remove this sentence altogether.

- After processing all values up to `grandTotal`, the combination at `coinsUsed[grandTotal]` will represent the most efficient solution.
- If no valid combination exists for `grandTotal`, an exception is thrown.

## Key Points
Copy link
Member

Choose a reason for hiding this comment

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

I think the core of this document should be focusing on the specifics of how the approach works. From that point of view, I think the Key Points heading/section is confusing as Time Complexity and Space Complexity isn't a core part of the implementation, but I do think we can certainly mention them. Perhaps we could remove the heading.

Suggested change
## Key Points

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure, I've removed this heading.


- **Space Complexity**: The space complexity is **O(n)** due to the list `coinsUsed`, which stores the most efficient coin combination for each total up to `grandTotal`.

- **Edge Cases**:
Copy link
Member

Choose a reason for hiding this comment

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

Suggest moving the points from the edge cases to be with your explanation as they are part of the implementation details. Idalso suggest separating them to follow the order of the code to make it easier to follow (i.e putting the point about checking if grandTotal is negative to the top and one about if no exact total to the bottom of the explanation).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've moved the first point to the start of the explanation as you suggested and removed the second point since it's already covered at the end, just before the 'Time Complexity' section.

## Conclusion

The dynamic programming approach provides an optimal solution for the change-making problem, ensuring that we minimize the number of coins used while efficiently solving the problem for any `grandTotal`.
However, it’s essential to consider the trade-offs in terms of memory usage and the time complexity when dealing with very large inputs.
Copy link
Member

Choose a reason for hiding this comment

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

I think we could remove this conclusion section. Similar to my earlier comment, I think the first sentence is redundant as all approaches have find the optimal solution to be valid. The second sentence would probably make more sense in a Which approach to use? section if we have more approaches.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree! I've removed the conclusion section altogether.

@jagdish-15
Copy link
Contributor Author

Sorry for all the back-and-forth! I've submitted it now—please take a look and let me know your thoughts.

Copy link
Member

@kahgoh kahgoh left a comment

Choose a reason for hiding this comment

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

No worries, thanks for preserevering! I think we're almost there!

Copy link
Member

@kahgoh kahgoh left a comment

Choose a reason for hiding this comment

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

Sorry, have some minor style comments that are similar to the ones on the Queen Attack PR


## Time and Space Complexity

- The time complexity of this approach is **O(n * m)**, where `n` is the `grandTotal` and `m` is the number of available coin denominations. This is because we iterate over all coin denominations for each amount up to `grandTotal`.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
- The time complexity of this approach is **O(n * m)**, where `n` is the `grandTotal` and `m` is the number of available coin denominations. This is because we iterate over all coin denominations for each amount up to `grandTotal`.
The time complexity of this approach is **O(n * m)**, where `n` is the `grandTotal` and `m` is the number of available coin denominations. This is because we iterate over all coin denominations for each amount up to `grandTotal`.


- The time complexity of this approach is **O(n * m)**, where `n` is the `grandTotal` and `m` is the number of available coin denominations. This is because we iterate over all coin denominations for each amount up to `grandTotal`.

- The space complexity is **O(n)** due to the list `coinsUsed`, which stores the most efficient coin combination for each total up to `grandTotal`.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
- The space complexity is **O(n)** due to the list `coinsUsed`, which stores the most efficient coin combination for each total up to `grandTotal`.
The space complexity is **O(n)** due to the list `coinsUsed`, which stores the most efficient coin combination for each total up to `grandTotal`.


## Explanation

1. **Initialize Coins Usage Tracker**:
Copy link
Member

Choose a reason for hiding this comment

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

Similar to one the comments on the Queen Attack approach, should this be a heading?

Suggested change
1. **Initialize Coins Usage Tracker**:
### Initialize Coins Usage Tracker

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This would make sense much more as a sub-heading!

- We create a list `coinsUsed`, where each index `i` stores the most efficient combination of coins that sum up to the value `i`.
- The list is initialized with an empty list at index `0`, as no coins are needed to achieve a total of zero.

2. **Iterative Dynamic Programming**:
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
2. **Iterative Dynamic Programming**:
### Iterative Dynamic Programming

- For each coin, we check if it can be part of the solution (i.e., if `coin <= i` and `coinsUsed[i - coin]` is a valid combination).
- If so, we generate a new combination by adding the current coin to the solution for `i - coin`. We then compare the size of this new combination with the existing best combination and keep the one with fewer coins.

3. **Result**:
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
3. **Result**:
### Result


1. **Initialize Coins Usage Tracker**:

- If the `grandTotal` is negative, an exception is thrown immediately.
Copy link
Member

Choose a reason for hiding this comment

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

If the making Initialize Coins Usage Tracker a heading the indent should be removed.

Suggested change
- If the `grandTotal` is negative, an exception is thrown immediately.
- If the `grandTotal` is negative, an exception is thrown immediately.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Indeed!

@jagdish-15
Copy link
Contributor Author

Sorry, have some minor style comments that are similar to the ones on the Queen Attack PR

No problem at all! I appreciate you taking the time to review. I'll address these style comments just like I did on the Queen Attack PR. Thanks for pointing them out!

@jagdish-15
Copy link
Contributor Author

I’ve updated the files based on your suggestions, and I think they look much better now. Thank you for the feedback—I’ll take it as a learning opportunity. Apologies for the delay in acting on the suggestions, similar to the Queen Attack PR.

Copy link
Member

@kahgoh kahgoh left a comment

Choose a reason for hiding this comment

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

Looks good now! Thanks for perservering with this one!

@kahgoh kahgoh merged commit 109a4c2 into exercism:main Nov 25, 2024
2 checks passed
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