-
Notifications
You must be signed in to change notification settings - Fork 20.1k
feat: Add 0/1 Knapsack Problem: Recursive and Tabulation (Bottom-Up DP) Implementations in Java along with their corresponding Tests #6421
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
Conversation
…orresponding tests
…orresponding tests
…orresponding tests
can u help me with the errors? |
@o000SAI000o thank you for your contribution to the repository. It looks like there are already several existing implementations of the Knapsack problem in the repository, and your solution appears to be very similar to them: https://github.com/TheAlgorithms/Java/blob/master/src/main/java/com/thealgorithms/dynamicprogramming/Knapsack.java https://github.com/TheAlgorithms/Java/blob/master/src/main/java/com/thealgorithms/dynamicprogramming/KnapsackMemoization.java Additionally, other variations of the Knapsack algorithm already exist in the repository. And the error appears because the filename does not match the public class name exactly, which is a strict rule in Java. |
thanks alot, i thought ZeroOneknapsack is a diffferent and popular version of it thts why i tried that, ans sorry for filename tht was by mistake |
knapsack tabulation is not in repo which is very time effient with a O(N) time complexity , is it ok if i add tht code in repo, it will be efficient to alot of learners. |
* | ||
* https://en.wikipedia.org/wiki/Knapsack_problem | ||
*/ | ||
public final class ZeroOneKnapsack { |
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.
What do you think about renaming all related classes to start with Knapsack?
This way, it will be easier to find and group all Knapsack implementations together in the project.
src/main/java/com/thealgorithms/dynamicprogramming/ZeroOneKnapsackTab.java
Show resolved
Hide resolved
int[][] dp = new int[n + 1][W + 1]; | ||
|
||
for (int i = 1; i <= n; i++) { | ||
int value = val[i - 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.
Consider using the final
keyword here and in the following lines where applicable - it helps improve code clarity and prevents unintended reassignment.
import org.junit.jupiter.api.Test; | ||
|
||
/** | ||
* Test class for {@code ZeroOneKnapsackTab}. |
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.
This comment is redundant, as it merely restates what is already clear from the class structure.
|
||
import org.junit.jupiter.api.Test; | ||
|
||
/** |
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.
The same suggestions for improvement also apply to this class.
/** | ||
* Tests the 0-1 Knapsack tabulation approach with known values. | ||
*/ | ||
@Test |
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.
Consider refactoring these test methods into a single parameterized test. Since all tests follow the same pattern calling compute
with different inputs and asserting expected outputs - using JUnit’s parameterized tests would reduce code duplication and improve maintainability. It will also make adding new test cases easier and keep the test class cleaner.
There are already many examples of parameterized tests in the repository.
* @return the maximum value that can be put in the knapsack | ||
*/ | ||
public static int compute(int[] val, int[] wt, int W, int n) { | ||
int[][] dp = new int[n + 1][W + 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.
It makes sense to add input validation here, such as checks for null or empty arrays. This helps prevent runtime errors and makes the code more robust and reliable. And tests for that.
* @param n the number of items | ||
* @return the maximum total value achievable within the given weight limit | ||
*/ | ||
public static int compute(int[] values, int[] weights, int capacity, int n) { |
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.
Making some parameters final is considered good practice.
It makes sense to add input validation here also.
@o000SAI000o the build needs to be fixed. |
sure ill fix the build |
1.Recursive Solution
-Uses a top-down approach without memoization.
-Highlights the decision process for including or excluding each item.
2.Tabulation (Bottom-Up DP) Solution
-Efficient dynamic programming approach using a 2D table.
-Includes a utility function to print the DP table.
-Demonstrates space-time tradeoff compared to recursion.
Both implementations follow consistent formatting, inline documentation, and are aligned with the repository's existing code structure.