5
\$\begingroup\$

Basically, find x and y given the product = xy and sum = x + y. I would like review on optimization, clean code, and complexity. It looks like O(1), but the while loop makes me wonder if it's something different.

/**
 *  x will always be less than or equal to y.
 */
final class Variables {
    private final int x;
    private final int y;

    Variables(int x, int y) {
        this.x = x;
        this.y = y;
    }

    public int getX() {
        return x;
    }

    public int getY() {
        return y;
    }

    @Override public String toString()  {
        return " x = " + x + " y = " + y;
    }
}

/**
 * @author SERVICE-NOW\ameya.patil
 */
public final class SimultaneousEquations {

    private SimultaneousEquations () {}

    /**
     * Provided we dont have positive product and -ve sum, 
     * return value would be the smallest posive integer of the variable.
     * 
     * @param product : product of x and y
     * @param sum     : sum of x and y
     * @return        : smallest positive integer of x and y.
     */
    private static int getX(int product, int sum) {
        assert product != 0;
        assert product < 0 || (product > 0 && sum > 0);

        int x = 1;
        while (x*x != (sum * x  - product)) {
            x++;
        }
        return x;
    }

    /**
     * Given a product and a sum, return two variables.
     * 
     * @param sum       : sum of two variables
     * @param product   : product of two variables
     */
    public static Variables getVariables(int product, int sum) {
        if (product == 0) {
            if (sum < 0) {
                return new Variables(sum, 0);
            } else {
                return new Variables(0, sum);
            }
        }

        if (product < 0) {
            int x = getX(product, sum);
            return new Variables(product/x, x);
        }

        if (sum < 0) {
            int x = -getX(product, -sum);
            return new Variables(product/x, x);
        } 

        int x = getX(product, sum);
        return new Variables(x, product/x);
    }    


    public static void main(String[] args) {
        // 0 10
        Variables v1 = SimultaneousEquations.getVariables(0, 10);
        boolean result1 = v1.getX() == 0 && v1.getY() == 10;
        System.out.println(result1);

        // 2 5
        Variables v2 = SimultaneousEquations.getVariables(10, 7);
        boolean result2 = v2.getX() == 2 && v2.getY() == 5;
        System.out.println(result2);

        // -2 5
        Variables v3 = SimultaneousEquations.getVariables(-10, 3);
        boolean result3 = v3.getX() == -2 && v3.getY() == 5;
        System.out.println(result3);

        // 2 -5
        Variables v4 = SimultaneousEquations.getVariables(-10, -3);
        boolean result4 = v4.getX() == -5 && v4.getY() == 2;
        System.out.println(result4);

        // -2 -5
        Variables v5 = SimultaneousEquations.getVariables(10, -7);
        boolean result5 = v5.getX() == -5 && v5.getY() == -2;
        System.out.println(result5);
    }
}
\$\endgroup\$
13
\$\begingroup\$

Before we start guessing, lets solve the equations properly. We can substitute the equations so that we get product = x · (sum - x) = -x² + sum·x ⇔ 0 = x² - sum·x + product, with y = sum - x.

The equation can be solved by x = sum/2 ± √(sum²/4 - product) (see Reduced Quadratic Equation). Note that we get zero, one or two distinct solutions. For example the special case of product = 0 leads to x = sum/2 ± sum/2 ⇒ {x = 0, x = sum}. It is up to you if you pick a specific solution or output both. We have no solutions in ℝ when the expression inside the square root is negative. However, there would still be complex solutions.

Restricting yourself to integers only isn't valid, and will miss some solutions. Consider product = 1, sum = 4. This is solved by x=3.73205, y=0.267949.

Note that as this equation can be solved arithmetically, there is no need for guessing via loops.


Because you resort to guessing, your code contains various bugs, e.g. it will fail to handle parameters with no solution like sum = 4, product = 5. Instead, you will go into an unterminated loop.

When unit-testing your code, you should be using a proper test harness instead of some test cases in a main. You might like JUnit. If you want to thoroughly test a method, do not only test a few simple cases:

  • Explicitly test failure cases, e.g. parameters with no solution.
  • Test especially large inputs to see if you correctly guarded against overflows (you didn't).
  • Test random input (and record the seed!). It is easy to verify the correctness of the solution. This fuzzy testing may hit cases you didn't consider.
  • Cover inputs around zero especially well, e.g. vary each parameter from -2 to 2 by 0.5-increments (after verifying manually that each parameter combination actually has a solution). Although this wouldn't hit another bug in your code, the combination 0, 0 is suspiciously absent. The combination 1, 0 would have exposed the bug that you don't handle inputs without solutions.
\$\endgroup\$
1
  • \$\begingroup\$ very useful input.. \$\endgroup\$ Nov 20 '13 at 9:35

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.