HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavaMinor

Finding if there is a subset that sums to a target value from an array of integers

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
fromtargetarrayfindingvalueintegersthatsumstheresubset

Problem

I came across this problem.


Problem Statement:


Given an array of ints, is it possible to choose a group of some of
the ints, beginning at the start index, such that the group sums to
the 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:

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.