-
Notifications
You must be signed in to change notification settings - Fork 20k
Add tests SumOfSubset
#5021
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
Add tests SumOfSubset
#5021
Conversation
@bhishma620 thanks for your contribution. This case is quite delicate - you are changing the code, which is not tested. Therefore I suggest the following approach:
Feel free to do each of the steps in separate pull-requests. Besides that, please make sure that all of the CI checks pass. If you have problems with In case of any questions: please do not hesitate - just ask them. |
Can you tell me how can i add tests ? |
b41962b
to
73fb041
Compare
src/main/java/com/thealgorithms/dynamicprogramming/Sum_Of_Subset.java
Outdated
Show resolved
Hide resolved
src/main/java/com/thealgorithms/dynamicprogramming/Sum_Of_SubsetTest.java
Outdated
Show resolved
Hide resolved
src/main/java/com/thealgorithms/dynamicprogramming/Sum_Of_SubsetTest.java
Outdated
Show resolved
Hide resolved
src/main/java/com/thealgorithms/dynamicprogramming/Sum_Of_SubsetTest.java
Outdated
Show resolved
Hide resolved
You did quite well. Please have a look on the review (this is only the 1st iteration).
I am not sure if there is a problem. But to be safe: always write test first before touching the implementation. Please make sure that the CI checks pass. |
c75c073
to
59f1060
Compare
Not able to pass Build test for Test file , how to resolve ? |
src/main/java/com/thealgorithms/dynamicprogramming/Sum_Of_Subset.java
Outdated
Show resolved
Hide resolved
fa99466
to
1d0970e
Compare
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 main step - adding tests - is done. Few small things are left.
You can also completely remove the main
method from Sum_Of_Subset
.
src/test/java/com/thealgorithms/dynamicprogramming/Sum_Of_SubsetTest.java
Outdated
Show resolved
Hide resolved
src/test/java/com/thealgorithms/dynamicprogramming/Sum_Of_SubsetTest.java
Outdated
Show resolved
Hide resolved
can you check now, i forgot to remove the main method from the SubOfSubset, is it necessary? |
src/main/java/com/thealgorithms/dynamicprogramming/SumOfSubset.java
Outdated
Show resolved
Hide resolved
src/test/java/com/thealgorithms/dynamicprogramming/SumOfSubsetTest.java
Outdated
Show resolved
Hide resolved
There is not reason here to have the |
src/test/java/com/thealgorithms/dynamicprogramming/SumOfSubsetTest.java
Outdated
Show resolved
Hide resolved
Can you check now? |
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.
@bhishma620: looks great! Please open a new pull-request with your original improvements of SumOfSubset
. You can mention this PR there (cf. docs).
Thanks!
This pull request introduces the implementation of the "Sum of Subset" algorithm in the file sum_of_subset.java. The Sum of Subset problem involves finding subsets of a given set whose sum matches a specific target sum.It Reduces time complexity from O(2^N) to O(n^2) by using extra space O(N).