D day - 1 year ago 76

Java Question

I have written a simple GCD function to implement Euclid's algorithm for computing the greatest common divisor gcd(m, n), which is the largest integer k dividing both m and n.

The function that I wrote successfully compiles:

`public static int gcd(int m, int n) {`

if (n == 0) return m;

return gcd(n, m%n);

}

However, I run into an error when I write a unit test on GCD:

`@Test public void gcdTest() {`

for (int m = 0; m < 15; m++) {

for (int n = 0; n < 15; n++) {

assertEquals("Divide m,n", m/n%m, Recursion.gcd(m,n));

}

}

}

The error comes in the 'assertEquals' line. I am unsure if maybe I am calculating this method incorrectly by writing m / n % m.

Any tips or suggestions? Thanks in advance.

Answer Source

Besides the "math" thing here - using loops within unit tests is something that you should not do immediately.

What I mean is: before you think about testcases that **iterate** and do multiple asserts within a loop, do things like

```
@Test
public void gcdTest1_1() {
assertThat(Recursion.gcd(1,1), is(1));
}
```

In other words: write simple testcases that test **one** thing only. And when the first one passes, write the next one. And then, when you are more confident, then consider such looping solution.

As that would might have given you that idea about dividing by 0 ... not being a thing to have in your tests!

Edit upon your comment: use see, the **core** idea of unit tests is that they help you find and fix bugs in your code under test. So looking at your example, the one big obstacle there is ... you are printing a string "m, n". That doesn't tell you anything. You already know that your variables are called m and n. You better print the **values** of m and n in case the assert fails.

Finally: I changed to **assertThat**; some other style of assert, that I find to lead to "more readable" code. When using that, you will have to use hamcrest matchers such as *is()* though (google is your friend here).