From 2365cae199199c9e91598abac2b64062804324f4 Mon Sep 17 00:00:00 2001 From: Yegor Kozlov Date: Fri, 16 Nov 2012 09:46:28 +0000 Subject: [PATCH] bugzilla 54137 - improved performance of DataFormatter with Fractions git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@1410269 13f79535-47bb-0310-9956-ffa450edef68 --- src/documentation/content/xdocs/status.xml | 1 + .../poi/ss/usermodel/DataFormatter.java | 80 ++++++++++++++++--- .../poi/ss/usermodel/TestDataFormatter.java | 39 +++++++++ 3 files changed, 108 insertions(+), 12 deletions(-) diff --git a/src/documentation/content/xdocs/status.xml b/src/documentation/content/xdocs/status.xml index c39312745..1e13ea806 100644 --- a/src/documentation/content/xdocs/status.xml +++ b/src/documentation/content/xdocs/status.xml @@ -34,6 +34,7 @@ + 54137 - improved performance of DataFormatter with Fractions 54099 - Ensure that CTHMerge and CTTcBorders go to poi-ooxml-schemas jar 54111 - Fixed extracting text from table cells in HSLF 52583 - add support for drop-down lists in doc to html convertion diff --git a/src/java/org/apache/poi/ss/usermodel/DataFormatter.java b/src/java/org/apache/poi/ss/usermodel/DataFormatter.java index cfa20914f..930f8b420 100644 --- a/src/java/org/apache/poi/ss/usermodel/DataFormatter.java +++ b/src/java/org/apache/poi/ss/usermodel/DataFormatter.java @@ -13,6 +13,10 @@ 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. + + 2012 - Alfresco Software, Ltd. + Alfresco Software has modified source of this file + The details of changes as svn diff can be found in svn at location root/projects/3rd-party/src ==================================================================== */ package org.apache.poi.ss.usermodel; @@ -248,6 +252,12 @@ public class DataFormatter { } private Format getFormat(double cellValue, int formatIndex, String formatStrIn) { +// // Might be better to separate out the n p and z formats, falling back to p when n and z are not set. +// // That however would require other code to be re factored. +// String[] formatBits = formatStrIn.split(";"); +// int i = cellValue > 0.0 ? 0 : cellValue < 0.0 ? 1 : 2; +// String formatStr = (i < formatBits.length) ? formatBits[i] : formatBits[0]; + String formatStr = formatStrIn; // Excel supports positive/negative/zero, but java // doesn't, so we need to do it specially @@ -364,10 +374,21 @@ public class DataFormatter { } // Excel supports fractions in format strings, which Java doesn't - if (!formatStr.contains("-") && - (formatStr.indexOf("#/#") >= 0 && formatStr.indexOf("#/#") == formatStr.lastIndexOf("#/#")) || - (formatStr.indexOf("?/?") >= 0 && formatStr.indexOf("?/?") == formatStr.lastIndexOf("?/?"))) { - return new FractionFormat(formatStr); + if (formatStr.indexOf("#/#") >= 0 || formatStr.indexOf("?/?") >= 0) { + // Strip custom text in quotes and escaped characters for now as it can cause performance problems in fractions. + String strippedFormatStr = formatStr.replaceAll("\\\\ ", " ").replaceAll("\\\\.", "").replaceAll("\"[^\"]*\"", " "); + + boolean ok = true; + for (String part: strippedFormatStr.split(";")) { + int indexOfFraction = indexOfFraction(part); + if (indexOfFraction == -1 || indexOfFraction != lastIndexOfFraction(part)) { + ok = false; + break; + } + } + if (ok) { + return new FractionFormat(strippedFormatStr); + } } if (numPattern.matcher(formatStr).find()) { @@ -380,6 +401,18 @@ public class DataFormatter { // TODO - when does this occur? return null; } + + private int indexOfFraction(String format) { + int i = format.indexOf("#/#"); + int j = format.indexOf("?/?"); + return i == -1 ? j : j == -1 ? i : Math.min(i, j); + } + + private int lastIndexOfFraction(String format) { + int i = format.lastIndexOf("#/#"); + int j = format.lastIndexOf("?/?"); + return i == -1 ? j : j == -1 ? i : Math.max(i, j); + } private Format createDateFormat(String pFormatStr, double cellValue) { String formatStr = pFormatStr; @@ -996,14 +1029,26 @@ public class DataFormatter { } public String format(Number num) { - double wholePart = Math.floor(num.doubleValue()); - double decPart = num.doubleValue() - wholePart; + + double doubleValue = num.doubleValue(); + + // Format may be p or p;n or p;n;z (okay we never get a z). + // Fall back to p when n or z is not specified. + String[] formatBits = str.split(";"); + int f = doubleValue > 0.0 ? 0 : doubleValue < 0.0 ? 1 : 2; + String str = (f < formatBits.length) ? formatBits[f] : formatBits[0]; + + double wholePart = Math.floor(Math.abs(doubleValue)); + double decPart = Math.abs(doubleValue) - wholePart; if (wholePart + decPart == 0) { return "0"; } - + if (doubleValue < 0.0) { + wholePart *= -1.0; + } + // Split the format string into decimal and fraction parts - String[] parts = str.split(" "); + String[] parts = str.replaceAll(" *", " ").split(" "); String[] fractParts; if (parts.length == 2) { fractParts = parts[1].split("/"); @@ -1017,11 +1062,12 @@ public class DataFormatter { } if (fractParts.length == 2) { + int fractPart1Length = Math.min(countHashes(fractParts[1]), 4); // Any more than 3 and we go around the loops for ever double minVal = 1.0; - double currDenom = Math.pow(10 , fractParts[1].length()) - 1d; + double currDenom = Math.pow(10 , fractPart1Length) - 1d; double currNeum = 0; - for (int i = (int)(Math.pow(10, fractParts[1].length())- 1d); i > 0; i--) { - for(int i2 = (int)(Math.pow(10, fractParts[1].length())- 1d); i2 > 0; i2--){ + for (int i = (int)(Math.pow(10, fractPart1Length)- 1d); i > 0; i--) { + for(int i2 = (int)(Math.pow(10, fractPart1Length)- 1d); i2 > 0; i2--){ if (minVal >= Math.abs((double)i2/(double)i - decPart)) { currDenom = i; currNeum = i2; @@ -1040,9 +1086,19 @@ public class DataFormatter { return result; } } else { - throw new IllegalArgumentException("Fraction must have 2 parts, found " + fractParts.length + " for fraction format " + str); + throw new IllegalArgumentException("Fraction must have 2 parts, found " + fractParts.length + " for fraction format " + this.str); } } + + private int countHashes(String format) { + int count = 0; + for (int i=format.length()-1; i >= 0; i--) { + if (format.charAt(i) == '#') { + count++; + } + } + return count; + } public StringBuffer format(Object obj, StringBuffer toAppendTo, FieldPosition pos) { return toAppendTo.append(format((Number)obj)); diff --git a/src/testcases/org/apache/poi/ss/usermodel/TestDataFormatter.java b/src/testcases/org/apache/poi/ss/usermodel/TestDataFormatter.java index b8c3a78a2..8bc4f0f26 100644 --- a/src/testcases/org/apache/poi/ss/usermodel/TestDataFormatter.java +++ b/src/testcases/org/apache/poi/ss/usermodel/TestDataFormatter.java @@ -13,6 +13,10 @@ 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. + + 2012 - Alfresco Software, Ltd. + Alfresco Software has modified source of this file + The details of changes as svn diff can be found in svn at location root/projects/3rd-party/src ==================================================================== */ package org.apache.poi.ss.usermodel; @@ -195,6 +199,41 @@ public class TestDataFormatter extends TestCase { assertEquals("321 1/3", dfUS.formatRawCellContents(321.321, -1, "# ?/?")); assertEquals("321 26/81", dfUS.formatRawCellContents(321.321, -1, "# ?/??")); assertEquals("26027/81", dfUS.formatRawCellContents(321.321, -1, "?/??")); + + // p;n;z;s parts + assertEquals( "321 1/3", dfUS.formatRawCellContents(321.321, -1, "# #/#;# ##/#;0;xxx")); + assertEquals("-321 1/3", dfUS.formatRawCellContents(-321.321, -1, "# #/#;# ##/#;0;xxx")); + assertEquals("0", dfUS.formatRawCellContents(0, -1, "# #/#;# ##/#;0;xxx")); +// assertEquals("0.0", dfUS.formatRawCellContents(0, -1, "# #/#;# ##/#;#.#;xxx")); // currently hard coded to 0 + + // Custom formats with text are not currently supported +// assertEquals("+ve", dfUS.formatRawCellContents(0, -1, "+ve;-ve;zero;xxx")); +// assertEquals("-ve", dfUS.formatRawCellContents(0, -1, "-ve;-ve;zero;xxx")); +// assertEquals("zero", dfUS.formatRawCellContents(0, -1, "zero;-ve;zero;xxx")); + + // Custom formats - check text is stripped, including multiple spaces + assertEquals("321 1/3", dfUS.formatRawCellContents(321.321, -1, "# #/#")); + assertEquals("321 1/3", dfUS.formatRawCellContents(321.321, -1, "#\" \" #/#")); + assertEquals("321 1/3", dfUS.formatRawCellContents(321.321, -1, "#\"FRED\" #/#")); + assertEquals("321 1/3", dfUS.formatRawCellContents(321.321, -1, "#\\ #/#")); + assertEquals("321 1/3", dfUS.formatRawCellContents(321.321, -1, "# \\q#/#")); + + // Cases that were very slow + assertEquals("321 1/3", dfUS.formatRawCellContents(321.321, -1, "0\" \"?/?;?/?")); // 0" "?/?;?/? - length of -ve part was used + assertEquals("321 1/3", dfUS.formatRawCellContents(321.321, -1, "0 \"#\"\\#\\#?/?")); // 0 "#"\#\#?/? - length of text was used + + assertEquals("321 295/919", dfUS.formatRawCellContents(321.321, -1, "# #/###")); + assertEquals("321 321/1000", dfUS.formatRawCellContents(321.321, -1, "# #/####")); // Code limits to #### as that is as slow as we want to get + assertEquals("321 321/1000", dfUS.formatRawCellContents(321.321, -1, "# #/##########")); + + // Not a valid fraction formats (too many #/# or ?/?) - hence the strange expected results + assertEquals("321 / ?/?", dfUS.formatRawCellContents(321.321, -1, "# #/# ?/?")); + assertEquals("321 / /", dfUS.formatRawCellContents(321.321, -1, "# #/# #/#")); + assertEquals("321 ?/? ?/?", dfUS.formatRawCellContents(321.321, -1, "# ?/? ?/?")); + assertEquals("321 ?/? / /", dfUS.formatRawCellContents(321.321, -1, "# ?/? #/# #/#")); + + // Where both p and n don't include a fraction, so cannot always be formatted + assertEquals("123", dfUS.formatRawCellContents(-123.321, -1, "0 ?/?;0")); } /**