patternjavaMinor
Finding if there is a subset that sums to a target value from an array of integers
Viewed 0 times
fromtargetarrayfindingvalueintegersthatsumstheresubset
Problem
I came across this problem.
Problem Statement:
Given an array of
the
the given target? However, with the additional constraint that all 6's
must be chosen.
Below is my code:
Please feel free to review, or if you have a better solution in mind which uses lesser number of
EDIT: The code was written using a plain text editor so it's less readable, for a readable version of code you may have a look here As there is already one review which explains about readability so request to review keeping the code optimization in view.
Problem Statement:
Given an array of
ints, is it possible to choose a group of some ofthe
ints, beginning at the start index, such that the group sums tothe given target? However, with the additional constraint that all 6's
must be chosen.
groupSum6(0, {5, 6, 2}, 8)→ true
groupSum6(0, {5, 6, 2}, 9)→ false
groupSum6(0, {5, 6, 2}, 7)→ false
Below is my code:
public boolean groupSum6(int start, int[] nums, int target) {
if(nums.length==0)
return target==0;
if(start==nums.length-1)
return target==nums[nums.length-1]||(target==0&&!(nums[nums.length-1]==6));
if(target==0&&nums[start]==6)
return false;
if(target==0&&!(nums[start]==6))
return groupSum6(start+1, nums, 0);
if(groupSum6(start+1, nums, target-nums[start])||nums[start]==6)
return groupSum6(start+1, nums, target-nums[start]);
return groupSum6(start+1, nums, target);
}Please feel free to review, or if you have a better solution in mind which uses lesser number of
ifs, you can share it in your review.EDIT: The code was written using a plain text editor so it's less readable, for a readable version of code you may have a look here As there is already one review which explains about readability so request to review keeping the code optimization in view.
Solution
This will be a solely style-focused review, because your code (don't take it personal) looks horrible...
SPACE to breathe:
Your code is cramped, and your poor operators and statments a sqeezed together like sardines in a tin.
Spaces are free! Use them!
Tabs to clarify:
You seem to have an aversion to indenting your code, or rather really indenting it. You indent by 2 spaces. That's a little thrifty. Use one tab or four spaces instead. This makes it easier to distinguish between levels, especially when you have large blocks of indented code in one method.
Braces to see:
Additionally you seem to dislike Braces to separate blocks. So here goes:
Wherever optional, still use braces!
Why? Because it's clear, consistent and less prone to bugs when you do.
Ctrl+Shift+F to format:
Use your IDE!!! Eclipse and Netbeans are both free, and both can do much of the formatting for you.
Your Code, after Applying just these three things:
I think this is much more readable
SPACE to breathe:
Your code is cramped, and your poor operators and statments a sqeezed together like sardines in a tin.
Spaces are free! Use them!
Tabs to clarify:
You seem to have an aversion to indenting your code, or rather really indenting it. You indent by 2 spaces. That's a little thrifty. Use one tab or four spaces instead. This makes it easier to distinguish between levels, especially when you have large blocks of indented code in one method.
Braces to see:
Additionally you seem to dislike Braces to separate blocks. So here goes:
Wherever optional, still use braces!
Why? Because it's clear, consistent and less prone to bugs when you do.
Ctrl+Shift+F to format:
Use your IDE!!! Eclipse and Netbeans are both free, and both can do much of the formatting for you.
Your Code, after Applying just these three things:
public boolean groupSum6(int start, int[] nums, int target) {
if (nums.length == 0) {
return target == 0;
}
if (start == nums.length - 1) {
return target == nums[nums.length - 1] ||
(target == 0 && !(nums[nums.length - 1] == 6));
}
if (target == 0 && nums[start] == 6) {
return false;
}
if (target == 0 && !(nums[start] == 6)) {
return groupSum6(start + 1, nums, 0);
}
if (groupSum6(start + 1, nums, target - nums[start]) || nums[start] == 6) {
return groupSum6(start + 1, nums, target - nums[start]);
}
return groupSum6(start + 1, nums, target);
}I think this is much more readable
Code Snippets
public boolean groupSum6(int start, int[] nums, int target) {
if (nums.length == 0) {
return target == 0;
}
if (start == nums.length - 1) {
return target == nums[nums.length - 1] ||
(target == 0 && !(nums[nums.length - 1] == 6));
}
if (target == 0 && nums[start] == 6) {
return false;
}
if (target == 0 && !(nums[start] == 6)) {
return groupSum6(start + 1, nums, 0);
}
if (groupSum6(start + 1, nums, target - nums[start]) || nums[start] == 6) {
return groupSum6(start + 1, nums, target - nums[start]);
}
return groupSum6(start + 1, nums, target);
}Context
StackExchange Code Review Q#56853, answer score: 3
Revisions (0)
No revisions yet.