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

Feat : Subset Sum using DP #202

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Suryac72
Copy link
Contributor

Hi @appgurueu @raklaptudirm please review my PR. I added subset sum problem using DP

const result = isSubsetSum(set, setLength, sum);
expect(result).toBe(false);
});
});
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is the indentation off?

* @param sum - The target sum to be achieved by the subset.
* @returns True if a subset with the specified sum exists; otherwise, false.
*/
export function isSubsetSum(set: number[], setLength: number, sum: number): boolean {
Copy link
Contributor

Choose a reason for hiding this comment

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

setLength is redundant, just use set.length.

Copy link
Contributor

Choose a reason for hiding this comment

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

I would like to suggest that the function prototype should be like this:

Suggested change
export function isSubsetSum(set: number[], setLength: number, sum: number): boolean {
export function isSubsetSum(set: number[], sum: number): boolean {

*/
export function isSubsetSum(set: number[], setLength: number, sum: number): boolean {

const dp: boolean[][] = new Array(setLength + 1);
Copy link
Contributor

Choose a reason for hiding this comment

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

Please try to give this a proper name, like subsetExists, and add some comment that explains that subsetExists[n][sum] is whether there exists a subset using up to the first n elements in set that sums up to sum.

}

for (let i = 0; i <= setLength; i++) {
dp[i][0] = true;
Copy link
Contributor

Choose a reason for hiding this comment

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

This base case could use a comment (something along the lines of "empty subset sums up to 0")


for (let i = 1; i <= setLength; i++) {
for (let j = 1; j <= sum; j++) {
if (set[i - 1] > j) {
Copy link
Contributor

Choose a reason for hiding this comment

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

This "recursion" formula should be explained.

Comment on lines +11 to +15
const dp: boolean[][] = new Array(setLength + 1);

for (let i = 0; i <= setLength; i++) {
dp[i] = new Array(sum + 1).fill(false);
}
Copy link
Contributor

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 this suggestion @appgurueu, @Suryac72

Suggested change
const dp: boolean[][] = new Array(setLength + 1);
for (let i = 0; i <= setLength; i++) {
dp[i] = new Array(sum + 1).fill(false);
}
// subsetExists[n][sum] indicates whether there exists a subset using up to the first n elements in set
// that sums up to sum.
const subsetExists: boolean[][] = Array.from({ length: set.length + 1 }, () => Array(sum + 1).fill(false));

Comment on lines +17 to +19
for (let i = 0; i <= setLength; i++) {
dp[i][0] = true;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
for (let i = 0; i <= setLength; i++) {
dp[i][0] = true;
}
// Base case: when sum is 0, there's always an empty subset.
for (let i = 0; i <= set.length; i++) {
subsetExists[i][0] = true;
}

Comment on lines +21 to +31
for (let i = 1; i <= setLength; i++) {
for (let j = 1; j <= sum; j++) {
if (set[i - 1] > j) {
dp[i][j] = dp[i - 1][j];
} else {
dp[i][j] = dp[i - 1][j] || dp[i - 1][j - set[i - 1]];
}
}
}

return dp[setLength][sum];
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
for (let i = 1; i <= setLength; i++) {
for (let j = 1; j <= sum; j++) {
if (set[i - 1] > j) {
dp[i][j] = dp[i - 1][j];
} else {
dp[i][j] = dp[i - 1][j] || dp[i - 1][j - set[i - 1]];
}
}
}
return dp[setLength][sum];
for (let currentIndex = 1; currentIndex <= set.length; currentIndex++) {
const currentElement = set[currentIndex - 1];
for (let currentSum = 1; currentSum <= sum; currentSum++) {
if (currentElement > currentSum) {
// If the current element is greater than the current sum,
// exclude the current element from the subset.
subsetExists[currentIndex][currentSum] = subsetExists[currentIndex - 1][currentSum];
} else {
// Either include or exclude the current element in the subset.
subsetExists[currentIndex][currentSum] = subsetExists[currentIndex - 1][currentSum] || subsetExists[currentIndex - 1][currentSum - currentElement];
}
}
}
return subsetExists[set.length][sum];

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.

None yet

3 participants