0

I have a Term class to define a polynomial:

public class Term
{
    final private int coef;
    final private int expo;

    private static Term zero, unit;

    static
    {
        try
        {
            zero = new Term(0, 0); // the number zero
            unit = new Term(1, 0); // the number one
        }
        catch (Exception e)
        {
            // constructor will not throw an exception here anyway
        }
    }

    /**
     * 
     * @param c
     *            The coefficient of the new term
     * @param e
     *            The exponent of the new term (must be non-negative)
     * @throws NegativeExponent
     */
    public Term(int c, int e) throws NegativeExponent
    {
        if (e < 0)
            throw new NegativeExponent();
        coef = c;
        expo = (coef == 0) ? 1 : e;
    }

    final public static Term Zero = zero;
    final public static Term Unit = unit;

    public boolean isConstant()
    {
        boolean isConst = false;
        if (this.expo == 0)
        {
            isConst = true;
        }
        return isConst;
    }
}

And I have the JUnit test as follows:

 /*
 *      const1      isConstant(zero)        =>      true    (0,0)
 *      const2      isConstant(unit)        =>      true    (1,0)
 *      const3      isConstant(0,5)         =>      true
 *      const4      isConstant(5,1)         =>      false
 */

 @Test 
 public void const1() throws TError { assertTrue(Term.Zero.isConstant()); }

 @Test 
 public void const2() throws TError { assertTrue(Term.Unit.isConstant()); }

 @Test 
 public void const3() throws TError { assertTrue(new Term(0,5).isConstant()); }

 @Test 
 public void const4() throws TError { assertFalse(new Term(5,1).isConstant()); }

Tests 2 and 4 pass as they should, but Tests 1 and 3 come up as failures and I cannot work out why, "Zero" is defining the polynomial as (0,0) and the other is defining it as (0,5). So by my thinking, the 1st one should give a green tick and the 3rd test should give a red cross as it has 5 as the exponent.

Any ideas?

jeff
  • 4,325
  • 16
  • 27
germainelol
  • 3,231
  • 15
  • 46
  • 82

2 Answers2

2

Please review your constructor:

public Term(int c, int e) throws NegativeExponent{
    if (e < 0) throw new NegativeExponent();
    coef = c;
    expo = (coef == 0) ? 1 : e;
}

When coef == 0, then exp is assigned with 1.

This makes zero as (0,1) not (0,0). this is the reason for test outcome as below:

    const1 -> false as expo = 1-->failure as expecting true
    const2 -> true as expo = 0 -->success as expecting true
    const3 -> false as expo = 5.-->failure as expecting true
    const4 -> false as expo = 1.-->success as expecting false

EDIT: To reverse fix your code to make the test cases pass, I think you may update your constructor as below:

public Term(int c, int e) throws NegativeExponent{
    if (e < 0) throw new NegativeExponent();
    coef = c;
    expo = (c == 0 && e != 0) ? 0 : e;
}

Here, I have updated the constructor to set expo as 0, if coef is 0 as any expo for coef 0 is immaterial.

Yogendra Singh
  • 33,927
  • 6
  • 63
  • 73
  • Thanks for the reply. The set of tests are what I am trying to achieve, I see why it is now giving the error as `Zero` would give me (0,1). But I don't see how this can be resolved in the constructor unless I am missing something. I think there is also an error with const3 as 5 should be false but that's not my concern. – germainelol Nov 16 '12 at 02:56
  • @user1828314: What do you mean by `But I don't see how this can be resolved in the constructor unless I am missing something.`?? What do you want to resolve. Your JUnit should be updated with `assertFalse` in `const1 & const3`. – Yogendra Singh Nov 16 '12 at 02:58
  • You said to review the constructor in order to resolve the error, but I'm not seeing how to fix the error at all unfortunately. I assumed maybe I was doing something wrong when defining `Zero` – germainelol Nov 16 '12 at 03:00
  • What I meant to say is that I am trying to achieve the JUnit tests, so I shouldn't be changing the `assertFalse` etc. My goal is to edit the Term class to fit the JUnit tests. – germainelol Nov 16 '12 at 03:02
  • @user1828314: I meant, try understanding your constructor which is changing the value of `expo` when coef is `0`. Why issue you want to fix? You mean test cases are correct and you want to reverse fix the code?? – Yogendra Singh Nov 16 '12 at 03:04
  • @user1828314: I got it now. You want to fix your code to make test cases pass. Let me add the fix in the answer. Wait for 2 minutes. – Yogendra Singh Nov 16 '12 at 03:05
  • @user1828314: Updated the answer with modified constructor. Check and let me know. – Yogendra Singh Nov 16 '12 at 03:19
  • Thanks, it seems to work with these tests and with all the previous tests of mine. Can I check with you the theory behind your code? If a polynomial has a coefficient of 0, then it is going to be 0x so regardless of what the exponent is it will result in being 0 anyway. Correct me if I am wrong please. – germainelol Nov 16 '12 at 03:25
  • @user1828314 Yes. If coef is `0` then any expo is immaterial. If you like the answer, don't forget to accept. – Yogendra Singh Nov 16 '12 at 03:31
1

Yogendra covered why your first test fails. Your const3() fails (for me at least) because new Term(0, 5) has a expo of 5, which gets replaced with 1 b/c of the coef, but 1 is still not 0 so new Term(0, 5) is NOT constant.

Not that you asked, but I'll also give you some general Java pointers for the future...

1) Use all caps for static constants

 public static final Term ZERO = new Term(0, 0);
 public static final Term UNIT = new Term(1, 0);

this is just a convention that the Java community expects.

2) Give your test cases meaningful names. One powerful use of test cases is that they can double as documentation of your assumptions at the time. Rather than const1() which tells a fellow developer nothing, use a descriptive name like zeroShouldBeAConstant(). This tells the next person who looks at your code that Zero should be considered a constant in this system.

3) Whenever you are returning a boolean variable, consider just returning the statement. Compare the function below to yours and tell me which one is more readable:

public boolean isConstant()
{
   return expo == 0;
}

It is less code and hopefully easier to read.

Good luck!

jeff
  • 4,325
  • 16
  • 27
  • Thanks for the pointers. Also implemented your short code for testing if coef is negative by using `return coef < 0;` beats my long winded method. – germainelol Nov 16 '12 at 03:09