diff --git a/benchmarks/NPOI.Benchmarks/Program.cs b/benchmarks/NPOI.Benchmarks/Program.cs index 22a3ae331..4f466d523 100644 --- a/benchmarks/NPOI.Benchmarks/Program.cs +++ b/benchmarks/NPOI.Benchmarks/Program.cs @@ -1,4 +1,4 @@ using BenchmarkDotNet.Running; using NPOI.Benchmarks; -BenchmarkSwitcher.FromAssembly(typeof(LargeExcelFileBenchmark).Assembly).Run(); \ No newline at end of file +BenchmarkSwitcher.FromAssembly(typeof(LargeExcelFileBenchmark).Assembly).Run(args); \ No newline at end of file diff --git a/main/SS/Formula/Eval/TwoOperandNumeric/MultiplyEval.cs b/main/SS/Formula/Eval/TwoOperandNumeric/MultiplyEval.cs index 923ad44a0..847ccb6d8 100644 --- a/main/SS/Formula/Eval/TwoOperandNumeric/MultiplyEval.cs +++ b/main/SS/Formula/Eval/TwoOperandNumeric/MultiplyEval.cs @@ -25,10 +25,18 @@ public class MultiplyEval : TwoOperandNumericOperation { public override double Evaluate(double d0, double d1) { - decimal dec0 = (decimal)d0; - decimal dec1 = (decimal)d1; + try + { + decimal dec0 = (decimal)d0; + decimal dec1 = (decimal)d1; - return decimal.ToDouble(dec0 * dec1); + return decimal.ToDouble(dec0 * dec1); + } + catch (System.OverflowException) + { + // Values too large for decimal — fall back to double arithmetic (matches Excel behavior) + return d0 * d1; + } } } } \ No newline at end of file diff --git a/main/SS/Formula/FormulaCellCache.cs b/main/SS/Formula/FormulaCellCache.cs index ed9ce780b..c57ede24f 100644 --- a/main/SS/Formula/FormulaCellCache.cs +++ b/main/SS/Formula/FormulaCellCache.cs @@ -17,7 +17,7 @@ limitations under the License. namespace NPOI.SS.Formula { - using System.Collections; + using System.Collections.Generic; public interface IEntryOperation { @@ -31,19 +31,18 @@ public interface IEntryOperation public class FormulaCellCache { - private readonly Hashtable _formulaEntriesByCell; + private readonly Dictionary _formulaEntriesByCell; public FormulaCellCache() { // assumes HSSFCell does not override HashCode or Equals, otherwise we need IdentityHashMap - _formulaEntriesByCell = new Hashtable(); + _formulaEntriesByCell = new Dictionary(); } public CellCacheEntry[] GetCacheEntries() { - FormulaCellCacheEntry[] result = new FormulaCellCacheEntry[_formulaEntriesByCell.Count]; - _formulaEntriesByCell.Values.CopyTo(result,0); + _formulaEntriesByCell.Values.CopyTo(result, 0); return result; } @@ -57,10 +56,8 @@ public void Clear() */ public FormulaCellCacheEntry Get(IEvaluationCell cell) { - if (_formulaEntriesByCell.ContainsKey(cell.IdentityKey)) - return (FormulaCellCacheEntry)_formulaEntriesByCell[cell.IdentityKey]; - else - return null; + _formulaEntriesByCell.TryGetValue(cell.IdentityKey, out FormulaCellCacheEntry entry); + return entry; } public void Put(IEvaluationCell cell, FormulaCellCacheEntry entry) @@ -70,17 +67,16 @@ public void Put(IEvaluationCell cell, FormulaCellCacheEntry entry) public FormulaCellCacheEntry Remove(IEvaluationCell cell) { - FormulaCellCacheEntry tmp = (FormulaCellCacheEntry)_formulaEntriesByCell[cell.IdentityKey]; - _formulaEntriesByCell.Remove(cell); + _formulaEntriesByCell.TryGetValue(cell.IdentityKey, out FormulaCellCacheEntry tmp); + _formulaEntriesByCell.Remove(cell.IdentityKey); return tmp; } public void ApplyOperation(IEntryOperation operation) { - IEnumerator i = _formulaEntriesByCell.Values.GetEnumerator(); - while (i.MoveNext()) + foreach (FormulaCellCacheEntry entry in _formulaEntriesByCell.Values) { - operation.ProcessEntry((FormulaCellCacheEntry)i.Current); + operation.ProcessEntry(entry); } } } diff --git a/main/SS/Formula/FormulaParser.cs b/main/SS/Formula/FormulaParser.cs index 15f876efb..992489b62 100644 --- a/main/SS/Formula/FormulaParser.cs +++ b/main/SS/Formula/FormulaParser.cs @@ -19,7 +19,6 @@ namespace NPOI.SS.Formula { using System; - using System.Collections; using System.Collections.Generic; using System.Globalization; using System.Text; @@ -1798,7 +1797,7 @@ private static bool IsArgumentDelimiter(int ch) private ParseNode[] Arguments() { //average 2 args per Function - ArrayList temp = new ArrayList(2); + List temp = new List(2); SkipWhite(); if (look == ')') { @@ -1834,7 +1833,7 @@ private ParseNode[] Arguments() throw expected("',' or ')'"); } } - ParseNode[] result = (ParseNode[])temp.ToArray(typeof(ParseNode)); + ParseNode[] result = temp.ToArray(); return result; } @@ -1989,7 +1988,7 @@ private static void CheckRowLengths(Object[][] values2d, int nColumns) private Object[] ParseArrayRow() { - ArrayList temp = new ArrayList(); + List temp = new List(); while (true) { temp.Add(ParseArrayItem()); @@ -2009,9 +2008,7 @@ private Object[] ParseArrayRow() break; } - Object[] result = new Object[temp.Count]; - result = temp.ToArray(); - return result; + return temp.ToArray(); } private Object ParseArrayItem() diff --git a/main/SS/Formula/FormulaUsedBlankCellSet.cs b/main/SS/Formula/FormulaUsedBlankCellSet.cs index bf39deeb0..6c50f2c6a 100644 --- a/main/SS/Formula/FormulaUsedBlankCellSet.cs +++ b/main/SS/Formula/FormulaUsedBlankCellSet.cs @@ -15,14 +15,12 @@ the License. You may obtain a copy of the License at limitations under the License. ==================================================================== */ -using System.Collections.Generic; - namespace NPOI.SS.Formula { using System; using System.Text; - using System.Collections; + using System.Collections.Generic; using NPOI.SS.Util; public class BookSheetKey @@ -202,11 +200,11 @@ public override String ToString() } } - private readonly Hashtable _sheetGroupsByBookSheet; + private readonly Dictionary _sheetGroupsByBookSheet; public FormulaUsedBlankCellSet() { - _sheetGroupsByBookSheet = new Hashtable(); + _sheetGroupsByBookSheet = new Dictionary(); } public void AddCell(IEvaluationWorkbook evalWorkbook, int bookIndex, int sheetIndex, int rowIndex, int columnIndex) @@ -219,19 +217,17 @@ private BlankCellSheetGroup GetSheetGroup(IEvaluationWorkbook evalWorkbook, int { BookSheetKey key = new BookSheetKey(bookIndex, sheetIndex); - BlankCellSheetGroup result = (BlankCellSheetGroup)_sheetGroupsByBookSheet[key]; - if (result == null) + if (!_sheetGroupsByBookSheet.TryGetValue(key, out BlankCellSheetGroup result)) { result = new BlankCellSheetGroup(evalWorkbook.GetSheet(sheetIndex).LastRowNum); - _sheetGroupsByBookSheet[key]= result; + _sheetGroupsByBookSheet[key] = result; } return result; } public bool ContainsCell(BookSheetKey key, int rowIndex, int columnIndex) { - BlankCellSheetGroup bcsg = (BlankCellSheetGroup)_sheetGroupsByBookSheet[key]; - if (bcsg == null) + if (!_sheetGroupsByBookSheet.TryGetValue(key, out BlankCellSheetGroup bcsg)) { return false; } diff --git a/main/SS/Formula/OperationEvaluatorFactory.cs b/main/SS/Formula/OperationEvaluatorFactory.cs index 509983a23..c61d440a7 100644 --- a/main/SS/Formula/OperationEvaluatorFactory.cs +++ b/main/SS/Formula/OperationEvaluatorFactory.cs @@ -19,7 +19,7 @@ namespace NPOI.SS.Formula { using System; - using System.Collections; + using System.Collections.Generic; using System.Reflection; using NPOI.SS.Formula.Eval; @@ -36,16 +36,16 @@ class OperationEvaluatorFactory { private static Type[] OPERATION_CONSTRUCTOR_CLASS_ARRAY = new Type[] { typeof(Ptg) }; - private static readonly Hashtable _instancesByPtgClass = InitialiseInstancesMap(); + private static readonly Dictionary _instancesByPtgClass = InitialiseInstancesMap(); private OperationEvaluatorFactory() { // no instances of this class } - private static Hashtable InitialiseInstancesMap() + private static Dictionary InitialiseInstancesMap() { - Hashtable m = new Hashtable(32); + Dictionary m = new Dictionary(32); Add(m, EqualPtg.instance, RelationalOperationEval.EqualEval); Add(m, GreaterEqualPtg.instance, RelationalOperationEval.GreaterEqualEval); Add(m, GreaterThanPtg.instance, RelationalOperationEval.GreaterThanEval); @@ -67,7 +67,7 @@ private static Hashtable InitialiseInstancesMap() return m; } - private static void Add(Hashtable m, OperationPtg ptgKey, Functions.Function instance) + private static void Add(Dictionary m, OperationPtg ptgKey, Functions.Function instance) { // make sure ptg has single private constructor because map lookups assume singleton keys ConstructorInfo[] cc = ptgKey.GetType().GetConstructors(); @@ -91,7 +91,7 @@ public static ValueEval Evaluate(OperationPtg ptg, ValueEval[] args, { throw new ArgumentException("ptg must not be null"); } - NPOI.SS.Formula.Functions.Function result = _instancesByPtgClass[ptg] as NPOI.SS.Formula.Functions.Function; + _instancesByPtgClass.TryGetValue(ptg, out NPOI.SS.Formula.Functions.Function result); FreeRefFunction udfFunc = null; if (result == null) diff --git a/main/SS/Formula/PTG/Ptg.cs b/main/SS/Formula/PTG/Ptg.cs index a6bb6974b..40bdbcdbd 100644 --- a/main/SS/Formula/PTG/Ptg.cs +++ b/main/SS/Formula/PTG/Ptg.cs @@ -20,11 +20,9 @@ namespace NPOI.SS.Formula.PTG using System; using System.IO; - using System.Runtime.Serialization.Formatters.Binary; - using System.Collections; + using System.Collections.Generic; using NPOI.Util; - using System.Collections.Generic; /** @@ -54,7 +52,7 @@ public abstract class Ptg : ICloneable */ public static Ptg[] ReadTokens(int size, ILittleEndianInput in1) { - ArrayList temp = new ArrayList(4 + size / 2); + List temp = new List(4 + size / 2); int pos = 0; bool hasArrayPtgs = false; while (pos < size) @@ -180,15 +178,14 @@ private static Ptg CreateBasePtg(byte id, ILittleEndianInput in1) } throw new Exception("Unexpected base token id (" + id + ")"); } - private static Ptg[] ToPtgArray(ArrayList l) + private static Ptg[] ToPtgArray(List l) { if (l.Count == 0) { return EMPTY_PTG_ARRAY; } - Ptg[] result = (Ptg[])l.ToArray(typeof(Ptg)); - return result; + return l.ToArray(); } /** * @return a distinct copy of this Ptg if the class is mutable, or the same instance diff --git a/main/SS/Formula/PlainCellCache.cs b/main/SS/Formula/PlainCellCache.cs index e09052d55..0db15ab70 100644 --- a/main/SS/Formula/PlainCellCache.cs +++ b/main/SS/Formula/PlainCellCache.cs @@ -19,7 +19,7 @@ namespace NPOI.SS.Formula { using System; - using System.Collections; + using System.Collections.Generic; using NPOI.Util; public class Loc @@ -93,11 +93,11 @@ public int BookIndex public class PlainCellCache { - private readonly Hashtable _plainValueEntriesByLoc; + private readonly Dictionary _plainValueEntriesByLoc; public PlainCellCache() { - _plainValueEntriesByLoc = new Hashtable(); + _plainValueEntriesByLoc = new Dictionary(); } public void Put(Loc key, PlainValueCellCacheEntry cce) { @@ -109,7 +109,8 @@ public void Clear() } public PlainValueCellCacheEntry Get(Loc key) { - return (PlainValueCellCacheEntry)_plainValueEntriesByLoc[key]; + _plainValueEntriesByLoc.TryGetValue(key, out PlainValueCellCacheEntry entry); + return entry; } public void Remove(Loc key) { diff --git a/testcases/main/SS/Formula/Eval/TestMultiplyEval.cs b/testcases/main/SS/Formula/Eval/TestMultiplyEval.cs index 83f5818a0..7a76870f0 100644 --- a/testcases/main/SS/Formula/Eval/TestMultiplyEval.cs +++ b/testcases/main/SS/Formula/Eval/TestMultiplyEval.cs @@ -47,9 +47,23 @@ public void TestBasic() System.Threading.Thread.CurrentThread.CurrentCulture = System.Globalization.CultureInfo.CreateSpecificCulture("en-US"); // issue #1055, use decimal to handle precision issue like (20000 * 0.000027 = 0.539999 in double) - Confirm(new NumberEval(20000), new NumberEval(0.000027), 0.54); + Confirm(new NumberEval(20000), new NumberEval(0.000027), 0.54); } - + + [Test] + public void TestLargeValuesOverflowDecimal() + { + // Values > ~7.9e28 overflow decimal; should fall back to double arithmetic + // Use direct assertions with relative tolerance since Confirm uses delta=0 + ValueEval[] args1 = { new NumberEval(1e29), new NumberEval(1e29) }; + double result1 = NumericFunctionInvoker.Invoke(EvalInstances.Multiply, args1, 0, 0); + ClassicAssert.AreEqual(1e58, result1, 1e58 * 1e-10, "Large positive multiply"); + + ValueEval[] args2 = { new NumberEval(-1e30), new NumberEval(1e30) }; + double result2 = NumericFunctionInvoker.Invoke(EvalInstances.Multiply, args2, 0, 0); + ClassicAssert.AreEqual(-1e60, result2, 1e60 * 1e-10, "Mixed sign large multiply"); + } + } } \ No newline at end of file diff --git a/testcases/main/SS/Formula/TestEvaluationCache.cs b/testcases/main/SS/Formula/TestEvaluationCache.cs index e8bec15be..fc3fddec9 100644 --- a/testcases/main/SS/Formula/TestEvaluationCache.cs +++ b/testcases/main/SS/Formula/TestEvaluationCache.cs @@ -847,6 +847,48 @@ public void TestPlainValueCache() ClassicAssert.AreEqual(8394753.0, summaryCell.NumericCellValue); } + /// + /// Verifies that FormulaCellCache.Remove() actually evicts entries. + /// Regression test for bug where Remove() used cell instead of cell.IdentityKey, + /// causing entries to never be removed from the cache. + /// + [Test] + public void TestFormulaCellCacheRemoveActuallyEvicts() + { + HSSFWorkbook wb = new HSSFWorkbook(); + ISheet sheet = wb.CreateSheet(); + IRow row = sheet.CreateRow(0); + + ICell cellA1 = row.CreateCell(0); + cellA1.SetCellFormula("1+1"); + ICell cellB1 = row.CreateCell(1); + cellB1.SetCellFormula("2+2"); + + IEvaluationCell evalCellA1 = HSSFEvaluationTestHelper.WrapCell(cellA1); + IEvaluationCell evalCellB1 = HSSFEvaluationTestHelper.WrapCell(cellB1); + + FormulaCellCache cache = new FormulaCellCache(); + FormulaCellCacheEntry entryA1 = new FormulaCellCacheEntry(); + FormulaCellCacheEntry entryB1 = new FormulaCellCacheEntry(); + + cache.Put(evalCellA1, entryA1); + cache.Put(evalCellB1, entryB1); + + // Both entries should be retrievable + ClassicAssert.AreSame(entryA1, cache.Get(evalCellA1)); + ClassicAssert.AreSame(entryB1, cache.Get(evalCellB1)); + ClassicAssert.AreEqual(2, cache.GetCacheEntries().Length); + + // Remove A1 and verify it's actually evicted + FormulaCellCacheEntry removed = cache.Remove(evalCellA1); + ClassicAssert.AreSame(entryA1, removed, "Remove should return the evicted entry"); + ClassicAssert.IsNull(cache.Get(evalCellA1), "Entry should be evicted after Remove"); + ClassicAssert.AreSame(entryB1, cache.Get(evalCellB1), "Other entries should be unaffected"); + ClassicAssert.AreEqual(1, cache.GetCacheEntries().Length); + + wb.Close(); + } + } } \ No newline at end of file