diff --git a/ClosedXML/Excel/CalcEngine/CalcEngineHelpers.cs b/ClosedXML/Excel/CalcEngine/CalcEngineHelpers.cs index c45be92..00d8a32 100644 --- a/ClosedXML/Excel/CalcEngine/CalcEngineHelpers.cs +++ b/ClosedXML/Excel/CalcEngine/CalcEngineHelpers.cs @@ -103,5 +103,27 @@ Debug.Assert(false, "failed to evaluate criteria in SumIf"); return false; } + + internal static bool ValueIsBlank(object value) + { + return + value == null || + value is string && ((string)value).Length == 0; + } + + /// + /// Get total count of cells in the specified range without initalizing them all + /// (which might cause serious performance issues on column-wide calculations). + /// + /// Expression referring to the cell range. + /// Total number of cells in the range. + internal static long GetTotalCellsCount(XObjectExpression rangeExpression) + { + var range = ((rangeExpression)?.Value as CellRangeReference)?.Range; + if (range == null) + return 0; + return (long)(range.LastColumn().ColumnNumber() - range.FirstColumn().ColumnNumber() + 1) * + (long)(range.LastRow().RowNumber() - range.FirstRow().RowNumber() + 1); + } } } diff --git a/ClosedXML/Excel/CalcEngine/Functions/MathTrig.cs b/ClosedXML/Excel/CalcEngine/Functions/MathTrig.cs index 233a7c8..4eb642e 100644 --- a/ClosedXML/Excel/CalcEngine/Functions/MathTrig.cs +++ b/ClosedXML/Excel/CalcEngine/Functions/MathTrig.cs @@ -340,20 +340,22 @@ { rangeValues.Add(value); } - var sumRangeValues = new List(); - foreach (var cell in ((CellRangeReference)sumRange.Value).Range.Cells()) + var sumRangeValues = new List(); + foreach (var value in sumRange) { - sumRangeValues.Add(cell); + sumRangeValues.Add(value); } // compute total var ce = new CalcEngine(); var tally = new Tally(); - for (var i = 0; i < Math.Min(rangeValues.Count, sumRangeValues.Count); i++) + for (var i = 0; i < Math.Max(rangeValues.Count, sumRangeValues.Count); i++) { - if (CalcEngineHelpers.ValueSatisfiesCriteria(rangeValues[i], criteria, ce)) + var targetValue = i < rangeValues.Count ? rangeValues[i] : string.Empty; + if (CalcEngineHelpers.ValueSatisfiesCriteria(targetValue, criteria, ce)) { - tally.AddValue(sumRangeValues[i].Value); + var value = i < sumRangeValues.Count ? sumRangeValues[i] : 0d; + tally.AddValue(value); } } @@ -401,7 +403,7 @@ foreach (var criteriaPair in criteriaRanges) { if (!CalcEngineHelpers.ValueSatisfiesCriteria( - criteriaPair.Item2[i], + i < criteriaPair.Item2.Count ? criteriaPair.Item2[i] : string.Empty, criteriaPair.Item1, ce)) { @@ -1028,4 +1030,4 @@ return m.Invert().mat; } } -} \ No newline at end of file +} diff --git a/ClosedXML/Excel/CalcEngine/Functions/Statistical.cs b/ClosedXML/Excel/CalcEngine/Functions/Statistical.cs index 958221e..06f720a 100644 --- a/ClosedXML/Excel/CalcEngine/Functions/Statistical.cs +++ b/ClosedXML/Excel/CalcEngine/Functions/Statistical.cs @@ -1,7 +1,7 @@ using ClosedXML.Excel.CalcEngine.Exceptions; using System; -using System.Collections; using System.Collections.Generic; +using System.Linq; namespace ClosedXML.Excel.CalcEngine { @@ -121,38 +121,40 @@ if ((p[0] as XObjectExpression)?.Value as CellRangeReference == null) throw new NoValueAvailableException("COUNTBLANK should have a single argument which is a range reference"); - var cnt = 0.0; var e = p[0] as XObjectExpression; + long totalCount = CalcEngineHelpers.GetTotalCellsCount(e); + long nonBlankCount = 0; foreach (var value in e) { - if (IsBlank(value)) - cnt++; + if (!CalcEngineHelpers.ValueIsBlank(value)) + nonBlankCount++; } - return cnt; - } - - internal static bool IsBlank(object value) - { - return - value == null || - value is string && ((string)value).Length == 0; + return 0d + totalCount - nonBlankCount; } private static object CountIf(List p) { CalcEngine ce = new CalcEngine(); var cnt = 0.0; - var ienum = p[0] as IEnumerable; + long processedCount = 0; + var ienum = p[0] as XObjectExpression; if (ienum != null) { + long totalCount = CalcEngineHelpers.GetTotalCellsCount(ienum); var criteria = (string)p[1].Evaluate(); foreach (var value in ienum) { if (CalcEngineHelpers.ValueSatisfiesCriteria(value, criteria, ce)) cnt++; + processedCount++; } + + // Add count of empty cells outside the used range if they match criteria + if (CalcEngineHelpers.ValueSatisfiesCriteria(string.Empty, criteria, ce)) + cnt += (totalCount - processedCount); } + return cnt; } @@ -160,15 +162,16 @@ { // get parameters var ce = new CalcEngine(); - int count = 0; + long count = 0; int numberOfCriteria = p.Count / 2; + long totalCount = 0; // prepare criteria-parameters: var criteriaRanges = new Tuple>[numberOfCriteria]; for (int criteriaPair = 0; criteriaPair < numberOfCriteria; criteriaPair++) { - var criteriaRange = p[criteriaPair * 2] as IEnumerable; + var criteriaRange = p[criteriaPair * 2] as XObjectExpression; var criterion = p[(criteriaPair * 2) + 1].Evaluate(); var criteriaRangeValues = new List(); foreach (var value in criteriaRange) @@ -179,26 +182,26 @@ criteriaRanges[criteriaPair] = new Tuple>( criterion, criteriaRangeValues); + + if (totalCount == 0) + totalCount = CalcEngineHelpers.GetTotalCellsCount(criteriaRange); } + long processedCount = 0; for (var i = 0; i < criteriaRanges[0].Item2.Count; i++) { - bool shouldUseValue = true; - - foreach (var criteriaPair in criteriaRanges) - { - if (!CalcEngineHelpers.ValueSatisfiesCriteria( - criteriaPair.Item2[i], - criteriaPair.Item1, - ce)) - { - shouldUseValue = false; - break; // we're done with the inner loop as we can't ever get true again. - } - } - - if (shouldUseValue) + if (criteriaRanges.All(criteriaPair => CalcEngineHelpers.ValueSatisfiesCriteria( + criteriaPair.Item2[i], criteriaPair.Item1, ce))) count++; + + processedCount++; + } + + // Add count of empty cells outside the used range if they match criteria + if (criteriaRanges.All(criteriaPair => CalcEngineHelpers.ValueSatisfiesCriteria( + string.Empty, criteriaPair.Item1, ce))) + { + count += (totalCount - processedCount); } // done diff --git a/ClosedXML/Excel/CalcEngine/Functions/Tally.cs b/ClosedXML/Excel/CalcEngine/Functions/Tally.cs index 8796b84..93abf67 100644 --- a/ClosedXML/Excel/CalcEngine/Functions/Tally.cs +++ b/ClosedXML/Excel/CalcEngine/Functions/Tally.cs @@ -80,7 +80,7 @@ if (numbersOnly) return NumericValuesInternal().Length; else - return _list.Count(o => !Statistical.IsBlank(o)); + return _list.Count(o => !CalcEngineHelpers.ValueIsBlank(o)); } private IEnumerable NumericValuesEnumerable() diff --git a/ClosedXML/Excel/CalcEngine/XLCalcEngine.cs b/ClosedXML/Excel/CalcEngine/XLCalcEngine.cs index 33bfb7f..da2f83b 100644 --- a/ClosedXML/Excel/CalcEngine/XLCalcEngine.cs +++ b/ClosedXML/Excel/CalcEngine/XLCalcEngine.cs @@ -82,7 +82,10 @@ // ** IEnumerable public IEnumerator GetEnumerator() { - return _range.Cells().Select(GetValue).GetEnumerator(); + var maxRow = Math.Min(_range.RangeAddress.LastAddress.RowNumber, _range.Worksheet.LastCellUsed().Address.RowNumber); + var maxCol = Math.Min(_range.RangeAddress.LastAddress.ColumnNumber, _range.Worksheet.LastCellUsed().Address.ColumnNumber); + using (var trimmedRange = (XLRangeBase)_range.Worksheet.Range(_range.FirstCell().Address, new XLAddress(maxRow, maxCol, false, false))) + return trimmedRange.CellValues().GetEnumerator(); } private Boolean _evaluating; diff --git a/ClosedXML/Excel/Cells/XLCell.cs b/ClosedXML/Excel/Cells/XLCell.cs index b0e368c..5e5eb14 100644 --- a/ClosedXML/Excel/Cells/XLCell.cs +++ b/ClosedXML/Excel/Cells/XLCell.cs @@ -368,6 +368,9 @@ var fA1 = FormulaA1; if (!String.IsNullOrWhiteSpace(fA1)) { + if (IsEvaluating) + throw new InvalidOperationException("Circular Reference"); + if (fA1[0] == '{') fA1 = fA1.Substring(1, fA1.Length - 2); diff --git a/ClosedXML/Excel/Ranges/IXLRangeBase.cs b/ClosedXML/Excel/Ranges/IXLRangeBase.cs index 4707327..0932cbf 100644 --- a/ClosedXML/Excel/Ranges/IXLRangeBase.cs +++ b/ClosedXML/Excel/Ranges/IXLRangeBase.cs @@ -1,4 +1,5 @@ using System; +using System.Collections; using System.Globalization; namespace ClosedXML.Excel diff --git a/ClosedXML/Excel/Ranges/XLRangeBase.cs b/ClosedXML/Excel/Ranges/XLRangeBase.cs index 2458574..5c9e661 100644 --- a/ClosedXML/Excel/Ranges/XLRangeBase.cs +++ b/ClosedXML/Excel/Ranges/XLRangeBase.cs @@ -1,6 +1,7 @@ using ClosedXML.Excel.Misc; using ClosedXML.Extensions; using System; +using System.Collections; using System.Collections.Generic; using System.Globalization; using System.Linq; @@ -285,6 +286,20 @@ return Cells(true); } + /// + /// Return the collection of cell values not initializing empty cells. + /// + public IEnumerable CellValues() + { + for (int ro = RangeAddress.FirstAddress.RowNumber; ro <= RangeAddress.LastAddress.RowNumber; ro++) + { + for (int co = RangeAddress.FirstAddress.ColumnNumber; co <= RangeAddress.LastAddress.ColumnNumber; co++) + { + yield return Worksheet.GetCellValue(ro, co); + } + } + } + public IXLRange Merge() { return Merge(true); diff --git a/ClosedXML/Excel/XLWorksheet.cs b/ClosedXML/Excel/XLWorksheet.cs index af55419..1868c12 100644 --- a/ClosedXML/Excel/XLWorksheet.cs +++ b/ClosedXML/Excel/XLWorksheet.cs @@ -1567,5 +1567,25 @@ else this.Cell(ro, co).SetValue(value); } + + /// + /// Get a cell value not initializing it if it has not been initialized yet. + /// + /// Row number + /// Column number + /// Current value of the specified cell. Empty string for non-initialized cells. + internal object GetCellValue(int ro, int co) + { + if (Internals.CellsCollection.MaxRowUsed < ro || + Internals.CellsCollection.MaxColumnUsed < co || + !Internals.CellsCollection.Contains(ro, co)) + return string.Empty; + + var cell = Worksheet.Internals.CellsCollection.GetCell(ro, co); + if (cell.IsEvaluating) + return string.Empty; + + return cell.Value; + } } } diff --git a/ClosedXML_Tests/Excel/CalcEngine/StatisticalTests.cs b/ClosedXML_Tests/Excel/CalcEngine/StatisticalTests.cs index 7a432e1..3f451f1 100644 --- a/ClosedXML_Tests/Excel/CalcEngine/StatisticalTests.cs +++ b/ClosedXML_Tests/Excel/CalcEngine/StatisticalTests.cs @@ -176,7 +176,7 @@ ws.Cell(4, 3).Value = "Yes"; ws.Cell(4, 4).Value = "Yes"; - Assert.AreEqual(expectedOutcome, (int)ws.Evaluate(formula)); + Assert.AreEqual(expectedOutcome, ws.Evaluate(formula)); } } @@ -329,6 +329,36 @@ Assert.AreEqual(2189.430863, value, tolerance); } + [Test] + [TestCase("COUNT(G:I,G:G,H:I)", 258d, Description = "COUNT overlapping columns")] + [TestCase("COUNT(6:8,6:6,7:8)", 30d, Description = "COUNT overlapping rows")] + [TestCase("COUNTBLANK(H:J)", 3145640d, Description = "COUNTBLANK columns")] + [TestCase("COUNTBLANK(7:9)", 49128d, Description = "COUNTBLANK rows")] + [TestCase("COUNT(1:1048576)", 216d, Description = "COUNT worksheet")] + [TestCase("COUNTBLANK(1:1048576)", 17179868831d, Description = "COUNTBLANK worksheet")] + [TestCase("SUM(H:J)", 20501.15d, Description = "SUM columns")] + [TestCase("SUM(4:5)", 85366.12d, Description = "SUM rows")] + [TestCase("SUMIF(G:G,50,H:H)", 24.98d, Description = "SUMIF columns")] + [TestCase("SUMIF(G23:G52,\"\",H3:H32)", 53.24d, Description = "SUMIF ranges")] + [TestCase("SUMIFS(H:H,G:G,50,I:I,\">900\")", 19.99d, Description = "SUMIFS columns")] + public void TallySkipsEmptyCells(string formulaA1, double expectedResult) + { + using (var wb = SetupWorkbook()) + { + var ws = wb.Worksheets.First(); + //Let's pre-initialize cells we need so they didn't affect the result + ws.Range("A1:J45").Style.Fill.BackgroundColor = XLColor.Amber; + ws.Cell("ZZ1000").Value = 1; + int initialCount = (ws as XLWorksheet).Internals.CellsCollection.Count; + + var actualResult = (double)ws.Evaluate(formulaA1); + int cellsCount = (ws as XLWorksheet).Internals.CellsCollection.Count; + + Assert.AreEqual(expectedResult, actualResult, tolerance); + Assert.AreEqual(initialCount, cellsCount); + } + } + private XLWorkbook SetupWorkbook() { var wb = new XLWorkbook();