aherbert commented on code in PR #138:
URL: https://github.com/apache/commons-numbers/pull/138#discussion_r1343236059


##########
commons-numbers-core/src/test/java/org/apache/commons/numbers/core/DDTest.java:
##########
@@ -78,6 +78,30 @@ void testZero() {
         Assertions.assertSame(DD.ZERO, DD.of(1.23).zero());
     }
 
+    @Test
+    void testIsOne() {
+        Assertions.assertTrue(DD.ONE.isOne());
+        Assertions.assertTrue(DD.of(0.5, 0).add(DD.of(0.5, 0)).isOne());
+        DD wide = DD.ofSum(1e300, 1e-300);
+        Assertions.assertTrue(wide.divide(wide).isOne());
+
+        Assertions.assertFalse(DD.ZERO.isOne());
+        Assertions.assertFalse(DD.of(0.5).isOne());
+    }
+
+    @Test
+    void testIsZero() {
+        Assertions.assertTrue(DD.ZERO.isZero());
+        Assertions.assertTrue(DD.of(-0.0).isZero());
+        Assertions.assertTrue(DD.of(0.5, 0).subtract(DD.of(0.5, 0)).isZero());
+        DD wide = DD.ofSum(1e300, 1e-300);
+        Assertions.assertTrue(wide.multiply(DD.of(0.0)).isZero());
+
+        Assertions.assertFalse(DD.ONE.isZero());
+        Assertions.assertFalse(DD.of(3.1415926).isZero());
+

Review Comment:
   Remove empty line.



##########
commons-numbers-core/src/test/java/org/apache/commons/numbers/core/DDTest.java:
##########
@@ -78,6 +78,30 @@ void testZero() {
         Assertions.assertSame(DD.ZERO, DD.of(1.23).zero());
     }
 
+    @Test
+    void testIsOne() {
+        Assertions.assertTrue(DD.ONE.isOne());
+        Assertions.assertTrue(DD.of(0.5, 0).add(DD.of(0.5, 0)).isOne());
+        DD wide = DD.ofSum(1e300, 1e-300);
+        Assertions.assertTrue(wide.divide(wide).isOne());
+
+        Assertions.assertFalse(DD.ZERO.isOne());
+        Assertions.assertFalse(DD.of(0.5).isOne());

Review Comment:
   For complete coverage we have to test the high part being 1 and the low part 
not 0:
   ```java
   Assertions.assertFalse(DD.ofSum(1.0, 1e-20).isOne());
   ```



##########
commons-numbers-core/src/test/java/org/apache/commons/numbers/core/DDTest.java:
##########
@@ -78,6 +78,30 @@ void testZero() {
         Assertions.assertSame(DD.ZERO, DD.of(1.23).zero());
     }
 
+    @Test
+    void testIsOne() {
+        Assertions.assertTrue(DD.ONE.isOne());
+        Assertions.assertTrue(DD.of(0.5, 0).add(DD.of(0.5, 0)).isOne());
+        DD wide = DD.ofSum(1e300, 1e-300);
+        Assertions.assertTrue(wide.divide(wide).isOne());
+
+        Assertions.assertFalse(DD.ZERO.isOne());
+        Assertions.assertFalse(DD.of(0.5).isOne());
+    }
+
+    @Test
+    void testIsZero() {
+        Assertions.assertTrue(DD.ZERO.isZero());
+        Assertions.assertTrue(DD.of(-0.0).isZero());
+        Assertions.assertTrue(DD.of(0.5, 0).subtract(DD.of(0.5, 0)).isZero());
+        DD wide = DD.ofSum(1e300, 1e-300);

Review Comment:
   I would change this from `wide` to `value`. Same for all the other tests.



##########
commons-numbers-core/src/main/java/org/apache/commons/numbers/core/DD.java:
##########
@@ -1924,6 +1924,20 @@ public DD pow(int n) {
         return computePow(x, xx, n);
     }
 
+    /** {@inheritDoc} */
+    @Override
+    public boolean isZero() {

Review Comment:
   Move these methods to the end of the class where the other overrides for the 
Field interfaces are defined, e.g. just below zero() and one().



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to