55419 and refactor SimpleFraction

git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@1514123 13f79535-47bb-0310-9956-ffa450edef68

src/java/org/apache/poi/ss/format/CellNumberFormatter.java

@@ -824,7 +824,7 @@ public class CellNumberFormatter extends CellFormatter {
                 n = (int) Math.round(fractional);
                 d = 1;
             } else {
-                Fraction frac = new Fraction(fractional, maxDenominator);
+                SimpleFraction frac = SimpleFraction.buildFractionMaxDenominator(fractional, maxDenominator);
                 n = frac.getNumerator();
                 d = frac.getDenominator();
             }
@@ -971,128 +971,6 @@ public class CellNumberFormatter extends CellFormatter {
         SIMPLE_NUMBER.formatValue(toAppendTo, value);
     }
 
-    /**
-     *  Based on org.apache.commons.math.fraction.Fraction from Apache Commons-Math.
-     *  YK: The only reason of having this inner class is to avoid dependency on the Commons-Math jar.
-     */
-    private static class Fraction {
-        /** The denominator. */
-        private final int denominator;
 
-        /** The numerator. */
-        private final int numerator;
-
-        /**
-         * Create a fraction given the double value and either the maximum error
-         * allowed or the maximum number of denominator digits.
-         *
-         * @param value the double value to convert to a fraction.
-         * @param epsilon maximum error allowed.  The resulting fraction is within
-         *        <code>epsilon</code> of <code>value</code>, in absolute terms.
-         * @param maxDenominator maximum denominator value allowed.
-         * @param maxIterations maximum number of convergents
-         * @throws RuntimeException if the continued fraction failed to
-         *         converge.
-         */
-        private Fraction(double value, double epsilon, int maxDenominator, int maxIterations)
-        {
-            long overflow = Integer.MAX_VALUE;
-            double r0 = value;
-            long a0 = (long)Math.floor(r0);
-            if (a0 > overflow) {
-                throw new IllegalArgumentException("Overflow trying to convert "+value+" to fraction ("+a0+"/"+1l+")");
-            }
-
-            // check for (almost) integer arguments, which should not go
-            // to iterations.
-            if (Math.abs(a0 - value) < epsilon) {
-                this.numerator = (int) a0;
-                this.denominator = 1;
-                return;
-            }
-
-            long p0 = 1;
-            long q0 = 0;
-            long p1 = a0;
-            long q1 = 1;
-
-            long p2;
-            long q2;
-
-            int n = 0;
-            boolean stop = false;
-            do {
-                ++n;
-                double r1 = 1.0 / (r0 - a0);
-                long a1 = (long)Math.floor(r1);
-                p2 = (a1 * p1) + p0;
-                q2 = (a1 * q1) + q0;
-                if ((p2 > overflow) || (q2 > overflow)) {
-                    throw new RuntimeException("Overflow trying to convert "+value+" to fraction ("+p2+"/"+q2+")");
-                }
-
-                double convergent = (double)p2 / (double)q2;
-                if (n < maxIterations && Math.abs(convergent - value) > epsilon && q2 < maxDenominator) {
-                    p0 = p1;
-                    p1 = p2;
-                    q0 = q1;
-                    q1 = q2;
-                    a0 = a1;
-                    r0 = r1;
-                } else {
-                    stop = true;
-                }
-            } while (!stop);
-
-            if (n >= maxIterations) {
-                throw new RuntimeException("Unable to convert "+value+" to fraction after "+maxIterations+" iterations");
-            }
-
-            if (q2 < maxDenominator) {
-                this.numerator = (int) p2;
-                this.denominator = (int) q2;
-            } else {
-                this.numerator = (int) p1;
-                this.denominator = (int) q1;
-            }
-
-        }
-
-        /**
-         * Create a fraction given the double value and maximum denominator.
-         * <p>
-         * References:
-         * <ul>
-         * <li><a href="http://mathworld.wolfram.com/ContinuedFraction.html">
-         * Continued Fraction</a> equations (11) and (22)-(26)</li>
-         * </ul>
-         * </p>
-         * @param value the double value to convert to a fraction.
-         * @param maxDenominator The maximum allowed value for denominator
-         * @throws RuntimeException if the continued fraction failed to
-         *         converge
-         */
-        public Fraction(double value, int maxDenominator)
-        {
-           this(value, 0, maxDenominator, 100);
-        }
-
-        /**
-         * Access the denominator.
-         * @return the denominator.
-         */
-        public int getDenominator() {
-            return denominator;
-        }
-
-        /**
-         * Access the numerator.
-         * @return the numerator.
-         */
-        public int getNumerator() {
-            return numerator;
-        }
-
-    }
 
 }

src/java/org/apache/poi/ss/format/SimpleFraction.java

@@ -0,0 +1,170 @@
+/* ====================================================================
+   Licensed to the Apache Software Foundation (ASF) under one or more
+   contributor license agreements.  See the NOTICE file distributed with
+   this work for additional information regarding copyright ownership.
+   The ASF licenses this file to You under the Apache License, Version 2.0
+   (the "License"); you may not use this file except in compliance with
+   the License.  You may obtain a copy of the License at
+
+       http://www.apache.org/licenses/LICENSE-2.0
+
+   Unless required by applicable law or agreed to in writing, software
+   distributed under the License is distributed on an "AS IS" BASIS,
+   WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+   See the License for the specific language governing permissions and
+   limitations under the License.
+*/
+package org.apache.poi.ss.format;
+
+public class SimpleFraction {
+
+
+    /** The denominator. */
+    private final int denominator;
+
+    /** The numerator. */
+    private final int numerator;
+    /**
+     * Create a fraction given a double value and a denominator.
+     * 
+     * @param val double value of fraction
+     * @param exactDenom the exact denominator
+     * @return
+     */
+    public static SimpleFraction buildFractionExactDenominator(double val, int exactDenom){
+        int num =  (int)Math.round(val*(double)exactDenom);
+        return new SimpleFraction(num,exactDenom);
+    }
+    
+    /**
+     * Create a fraction given the double value and either the maximum error
+     * allowed or the maximum number of denominator digits.
+     *
+     * @param value the double value to convert to a fraction.
+     * @param maxDenominator maximum denominator value allowed.
+     * 
+     * @throws RuntimeException if the continued fraction failed to
+     *      converge.
+     * @throws IllegalArgumentException if value > Integer.MAX_VALUE
+     */
+    public static SimpleFraction buildFractionMaxDenominator(double value, int maxDenominator){
+        return buildFractionMaxDenominator(value, 0, maxDenominator, 100);
+    }
+    /**
+     * Create a fraction given the double value and either the maximum error
+     * allowed or the maximum number of denominator digits.
+     * <p>
+     * References:
+     * <ul>
+     * <li><a href="http://mathworld.wolfram.com/ContinuedFraction.html">
+     * Continued Fraction</a> equations (11) and (22)-(26)</li>
+     * </ul>
+     * </p>
+     *
+     *  Based on org.apache.commons.math.fraction.Fraction from Apache Commons-Math.
+     *  YK: The only reason of having this class is to avoid dependency on the Commons-Math jar.
+     *
+     * @param value the double value to convert to a fraction.
+     * @param epsilon maximum error allowed.  The resulting fraction is within
+     *        <code>epsilon</code> of <code>value</code>, in absolute terms.
+     * @param maxDenominator maximum denominator value allowed.
+     * @param maxIterations maximum number of convergents
+     * @throws RuntimeException if the continued fraction failed to
+     *         converge.
+     * @throws IllegalArgumentException if value > Integer.MAX_VALUE
+     */
+    private static SimpleFraction buildFractionMaxDenominator(double value, double epsilon, int maxDenominator, int maxIterations)
+    {
+        long overflow = Integer.MAX_VALUE;
+        double r0 = value;
+        long a0 = (long)Math.floor(r0);
+        if (a0 > overflow) {
+            throw new IllegalArgumentException("Overflow trying to convert "+value+" to fraction ("+a0+"/"+1l+")");
+        }
+
+        // check for (almost) integer arguments, which should not go
+        // to iterations.
+        if (Math.abs(a0 - value) < epsilon) {
+            return new SimpleFraction((int)a0, 1);
+        }
+
+        long p0 = 1;
+        long q0 = 0;
+        long p1 = a0;
+        long q1 = 1;
+
+        long p2;
+        long q2;
+
+        int n = 0;
+        boolean stop = false;
+        do {
+            ++n;
+            double r1 = 1.0 / (r0 - a0);
+            long a1 = (long)Math.floor(r1);
+            p2 = (a1 * p1) + p0;
+            q2 = (a1 * q1) + q0;
+            //MATH-996/POI-55419
+            if (epsilon == 0.0f && maxDenominator > 0 && Math.abs(q2) > maxDenominator &&
+                    Math.abs(q1) < maxDenominator){
+
+                return new SimpleFraction((int)p1, (int)q1);
+            }
+            if ((p2 > overflow) || (q2 > overflow)) {
+                throw new RuntimeException("Overflow trying to convert "+value+" to fraction ("+p2+"/"+q2+")");
+            }
+
+            double convergent = (double)p2 / (double)q2;
+            if (n < maxIterations && Math.abs(convergent - value) > epsilon && q2 < maxDenominator) {
+                p0 = p1;
+                p1 = p2;
+                q0 = q1;
+                q1 = q2;
+                a0 = a1;
+                r0 = r1;
+            } else {
+                stop = true;
+            }
+        } while (!stop);
+
+        if (n >= maxIterations) {
+            throw new RuntimeException("Unable to convert "+value+" to fraction after "+maxIterations+" iterations");
+        }
+
+        if (q2 < maxDenominator) {
+            return new SimpleFraction((int) p2, (int)q2);
+        } else {
+            return new SimpleFraction((int)p1, (int)q1);
+        }
+
+    }
+
+    /**
+     * Create a fraction given a numerator and denominator.
+     * @param numerator
+     * @param denominator maxDenominator The maximum allowed value for denominator
+     */
+    public SimpleFraction(int numerator, int denominator)
+    {
+        this.numerator = numerator;
+        this.denominator = denominator;
+    }
+
+    /**
+     * Access the denominator.
+     * @return the denominator.
+     */
+    public int getDenominator() {
+        return denominator;
+    }
+
+    /**
+     * Access the numerator.
+     * @return the numerator.
+     */
+    public int getNumerator() {
+        return numerator;
+    }
+
+}
+

src/java/org/apache/poi/ss/usermodel/FractionFormat.java

@@ -22,6 +22,7 @@ import java.text.ParsePosition;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
 
+import org.apache.poi.ss.format.SimpleFraction;
 import org.apache.poi.ss.formula.eval.NotImplementedException;
 
 /**
@@ -35,7 +36,7 @@ import org.apache.poi.ss.formula.eval.NotImplementedException;
  *  If further uses for Commons Math are found, we will consider adding it as a dependency.
  *  For now, we have in-lined the one method to keep things simple.</p>
  */
-/* One question remains...is the value of epsilon in calcFractionMaxDenom reasonable? */
+
 @SuppressWarnings("serial")
 public class FractionFormat extends Format {
     private final static Pattern DENOM_FORMAT_PATTERN = Pattern.compile("(?:(#+)|(\\d+))");
@@ -113,7 +114,6 @@ public class FractionFormat extends Format {
         }
         
         //this is necessary to prevent overflow in the maxDenom calculation
-        //stink1
         if (wholePart+(int)decPart == wholePart+decPart){
             
             StringBuilder sb = new StringBuilder();
@@ -128,11 +128,11 @@ public class FractionFormat extends Format {
         try{
             //this should be the case because of the constructor
             if (exactDenom > 0){
-                fract = calcFractionExactDenom(decPart, exactDenom);
+                fract = SimpleFraction.buildFractionExactDenominator(decPart, exactDenom);
             } else {
-                fract = calcFractionMaxDenom(decPart, maxDenom);
+                fract = SimpleFraction.buildFractionMaxDenominator((double)decPart, maxDenom);
             }
-        } catch (SimpleFractionException e){
+        } catch (RuntimeException e){
             e.printStackTrace();
             return Double.toString(doubleValue);
         }
@@ -176,96 +176,4 @@ public class FractionFormat extends Format {
         throw new NotImplementedException("Reverse parsing not supported");
     }
    
-    private SimpleFraction calcFractionMaxDenom(double value, int maxDenominator) 
-            throws SimpleFractionException{
-        /*
-         * Lifted wholesale from org.apache.math.fraction.Fraction 2.2
-         */
-        double epsilon = 0.000000000001f;
-        int maxIterations = 100;
-        long overflow = Integer.MAX_VALUE;
-        double r0 = value;
-        long a0 = (long)Math.floor(r0);
-        if (Math.abs(a0) > overflow) {
-            throw new SimpleFractionException(
-                    String.format("value > Integer.MAX_VALUE: %d.", a0));
-        }
-
-        // check for (almost) integer arguments, which should not go
-        // to iterations.
-        if (Math.abs(a0 - value) < epsilon) {
-            return new SimpleFraction((int) a0, 1);
-        }
-
-        long p0 = 1;
-        long q0 = 0;
-        long p1 = a0;
-        long q1 = 1;
-
-        long p2 = 0;
-        long q2 = 1;
-
-        int n = 0;
-        boolean stop = false;
-        do {
-            ++n;
-            double r1 = 1.0 / (r0 - a0);
-            long a1 = (long)Math.floor(r1);
-            p2 = (a1 * p1) + p0;
-            q2 = (a1 * q1) + q0;
-            if ((Math.abs(p2) > overflow) || (Math.abs(q2) > overflow)) {
-                throw new SimpleFractionException(
-                        String.format("Greater than overflow in loop %f, %d, %d", value, p2, q2));
-            }
-
-            double convergent = (double)p2 / (double)q2;
-            if (n < maxIterations && Math.abs(convergent - value) > epsilon && q2 < maxDenominator) {
-                p0 = p1;
-                p1 = p2;
-                q0 = q1;
-                q1 = q2;
-                a0 = a1;
-                r0 = r1;
-            } else {
-                stop = true;
-            }
-        } while (!stop);
-
-        if (n >= maxIterations) {
-            throw new SimpleFractionException("n greater than max iterations " + value + " : " + maxIterations);
-        }
-
-        if (q2 < maxDenominator) {
-            return new SimpleFraction((int) p2, (int) q2);
-        } else {
-            return new SimpleFraction((int) p1, (int) q1);
-        }
-    }
-
-    private SimpleFraction calcFractionExactDenom(double val, int exactDenom){
-        int num =  (int)Math.round(val*(double)exactDenom);
-        return new SimpleFraction(num,exactDenom);
-    }
-
-    private class SimpleFraction {
-        private final int num;
-        private final int denom;
-
-        public SimpleFraction(int num, int denom) {
-            this.num = num;
-            this.denom = denom;
-        }
-
-        public int getNumerator() {
-            return num;
-        }
-        public int getDenominator() {
-            return denom;
-        }
-    }
-    private class SimpleFractionException extends Throwable{
-        private SimpleFractionException(String message){
-            super(message);
-        }
-    }
 }

src/testcases/org/apache/poi/ss/format/TestCellFormat.java

@@ -825,4 +825,15 @@ public class TestCellFormat extends TestCase {
         
     }
     
+    public void testSimpleFractionFormat() {
+        CellFormat cf1 = CellFormat.getInstance("# ?/?");
+        // Create a workbook, row and cell to test with
+        Workbook wb = new HSSFWorkbook();
+        Sheet sheet = wb.createSheet();
+        Row row = sheet.createRow(0);
+        Cell cell = row.createCell(0);
+        cell.setCellValue(123456.6);
+        System.out.println(cf1.apply(cell).text);
+        assertEquals("123456 3/5", cf1.apply(cell).text);
+    }
 }