JUnit testing for Calculator in Java

Question

I would also like to write some simple unit tests for my CalculatorEngine class. I had to make some of my private methods as package-private over this. How is it usually better to do it - to enhance access modifier or to make inner test class?

And what another more complicated cases it's better to test?

My CalculatorEngineTest class is:

import org.junit.Before;
import org.junit.Test;

import static junit.framework.Assert.assertTrue;
import static junit.framework.TestCase.assertEquals;

public class CalculatorEngineTest {

    CalculatorEngine calcEngine = new CalculatorEngine();

    @Before
    public void runBeforeEveryTest() {
        calcEngine.currentTotal = 0;
    }

    @Test
    public void testGetTotalStringInt() throws Exception {
        calcEngine.currentTotal = 50;
        assertTrue(calcEngine.currentTotal % 1.0 == 0);
    }

    @Test
    public void testGetTotalStringDouble() throws Exception {
        calcEngine.equal("50.5");
        assertTrue(calcEngine.currentTotal % 1.0 != 0);
    }

    @Test
    public void testEqual() throws Exception {
        calcEngine.equal("20");
        assertEquals(calcEngine.currentTotal, 20.0);
    }

    @Test
    public void testAdd() throws Exception {
        calcEngine.add(20);
        assertEquals(calcEngine.currentTotal, 20.0);

    }

    @Test
    public void testSubtract() throws Exception {
        calcEngine.subtract(20);
        assertEquals(calcEngine.currentTotal, -20.0);
    }

    @Test
    public void testMultiplyByZero() throws Exception {
        calcEngine.multiply(10);
        assertEquals(calcEngine.currentTotal, 0.0);
    }

    @Test
    public void testDivideByZero() throws Exception {
        calcEngine.divide(10);
        assertEquals(calcEngine.currentTotal, 0.0);
    }

    @Test
    public void testMultiply() throws Exception {
        calcEngine.add(10);
        calcEngine.multiply(10);
        assertEquals(calcEngine.currentTotal, 100.0);
    }

    @Test
    public void testDivide() throws Exception {
        calcEngine.add(10);
        calcEngine.divide(10);
        assertEquals(calcEngine.currentTotal, 1.0);
    }
}

h.j.k. · Accepted Answer · 2015-06-10 01:19:04Z

Do you mean the default access modifier (i.e. none specified) by package-private? ~~If you can offer your CalculatorEngine's code as another review then I'm sure some of us can give you good-quality advice there.~~ As pointed out by OP, it's actually in this question: Simple calculator in Java using Swing and AWT. :)

Running down your list of tests...

First, why do all your tests throws Exception? Are they expected to? If they are, shouldn't you want JUnit to catch it and assert the right Exception is thrown? Your tests should be written in the minimal way possible, as you can also think of it as a form of documentation describing what your methods can or cannot do. If another developer (e.g. "You" six months down the road) sees the throws Exception, they may be mislead into thinking "an Exception may be thrown here... somehow.".

@Before
public void runBeforeEveryTest() {
    calcEngine.currentTotal = 0;
}

Your CalculatorEngine should probable have a reset() method for this. Manipulating a field directly in this way is prone to errors.

@Test
public void testGetTotalStringDouble() throws Exception {
    calcEngine.equal("50.5");
    assertTrue(calcEngine.currentTotal % 1.0 != 0);
}

So what does this do? Presumably, equal("50.5") sets currentTotal as 50.5, and we're testing that it's not an integer...? I wouldn't be worried about this. If the intention is to test whether equal() accepts a decimal-number-as-a-String, test that directly.

@Test
public void testMultiplyByZero() throws Exception {
    calcEngine.multiply(10);
    assertEquals(calcEngine.currentTotal, 0.0);
}

@Test
public void testDivideByZero() throws Exception {
    calcEngine.divide(10);
    assertEquals(calcEngine.currentTotal, 0.0);
}

The method names are misleading here, because the argument you use isn't zero, and your other method names do not read as e.g. addToZero(). Originally, I thought the division test was a division by zero test, and wanted to point out that doesn't give 0.0... Testing should involve edge cases too.

@Test
public void testMultiply() throws Exception {
    calcEngine.add(10);
    calcEngine.multiply(10);
    assertEquals(calcEngine.currentTotal, 100.0);
}

@Test
public void testDivide() throws Exception {
    calcEngine.add(10);
    calcEngine.divide(10);
    assertEquals(calcEngine.currentTotal, 1.0);
}

Ok, so now these are the 'normal' multiplication and division tests... It's still slightly odd that you have to do an 'add' first, which seems to be a merely convenient method (pun intended) of 'initializing' currentTotal as 10. You may want to look into this particular API design too.

Here is my CalculatorEngine class: github.com/DimaSanKiev/Calculator/blob/master/src/main/… Yes, I meant default access modifier. Thank you for your suggestions. — DimaSan, Jun 10 '15 at 1:14
Ah... that CalculatorEngine, which you have previously covered in your first question here in CR... Do consider the suggestions there for improving it. :) — h.j.k., Jun 10 '15 at 1:17
Yes that's it! Now I making some refactoring, based on given suggestions and also want to prepare some tests for it. — DimaSan, Jun 10 '15 at 1:21
By the way if I add private CalculatorView calcView = new CalculatorView(); field in my CalculatorEngineTest class to have access to actionClear() method, every time I run tests, all the GUI is also builds. Tell me please how do you think is it better to change design by adding all that logic to CalculatorEngine class? — DimaSan, Jun 10 '15 at 1:42
@DimaSan your 'view' should not belong to the 'engine'... a bit of shameless plug, but you can see how I separate the GUI implementation here from the CLI implementation here. — h.j.k., Jun 10 '15 at 5:37

maaartinus · Answer 2 · 2015-06-10 01:25:53Z

Just a small addendum to h.j.k.'s review:

I would also like to write some simple unit tests for my CalculatorEngine class. I had to make some of my private methods as package-private over this. How is it usually better to do it - to enhance access modifier or to make inner test class?

This is quite common, and this is also the reason while tests get placed into the same package. There's even an annotation in Guava called @VisibleForTesting.

However, private methods rarely need to be tested and this tells me that's something wrong with your class under test (getting such feedback is a very important bonus from testing!).

As your engine exposes operations on Strings, you should test them instead (testing helper methods is fine, but it's the less important part).

CalculatorEngine calcEngine = new CalculatorEngine();

Make it private. While the test methods must be (for whatever crazy reason) public, this really needn't be.

assertTrue(calcEngine.currentTotal % 1.0 == 0);

This is too weak. Obviously,

assertEquals(50, calcEngine.currentTotal);

sounds bad because of round-off errors (though it would work in this case), but there's a method allowing an absolute error:

assertEquals(50, calcEngine.currentTotal, 1e-6);

calcEngine.equal("50.5");

This is a misnomer, it should be set or setSomething.

In your engine there are things, which simply make no sense like

void add(double number) {
    currentTotal += number % 1.0 == 0 ? (int) number : number;
}

Overall it's strange, as your convert a specific operation like add(String) into a general one using Operator.ADD and then back again to add(double).

Thank you for this useful addendum. Sure this construction is over-engineered, I only use it because I need to get number as integer (without dot .) if it doesn't have remainder. Otherwise I got numbers like 100.0. A kind of "crutch". — DimaSan, Jun 10 '15 at 1:37
@DimaSan Concerning getTotalString, it's understandable. But the operations don't need it, do they? — maaartinus, Jun 10 '15 at 12:50

asked	9 months ago
viewed	1519 times
active	9 months ago

current community

your communities

more stack exchange communities

JUnit testing for Calculator in Java

2 Answers 2

Your Answer

Not the answer you're looking for? Browse other questions tagged java unit-testing junit or ask your own question.

Linked

Hot Network Questions

current community

your communities

more stack exchange communities

JUnit testing for Calculator in Java

2 Answers 2

Your Answer

Sign up or log in

Post as a guest

Not the answer you're looking for? Browse other questions tagged java unit-testing junit or ask your own question.

Linked

Related

Hot Network Questions