Merge pull request #1461 from antony-liu/poi/Bug-56822

POI Bug #56822  fix COUNTIFS()

main/SS/Formula/Functions/Baseifs.cs

@@ -1,4 +1,23 @@
-using NPOI.SS.Formula.Eval;
+/*
+ *  ====================================================================
+ *    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.
+ * ====================================================================
+ */
+
+using NPOI.SS.Formula.Eval;
 using System;
 using System.Collections.Generic;
 using System.Linq;
@@ -7,102 +26,109 @@
 
 namespace NPOI.SS.Formula.Functions
 {
+    /// <summary>
+    /// Base class for SUMIFS() and COUNTIFS() functions, as they share much of the same logic, 
+    /// the difference being the source of the totals.
+    /// </summary>
     public abstract class Baseifs : FreeRefFunction
     {
-        public abstract bool HasInitialRange();
+        /// <summary>
+        /// Implementations must be stateless.
+        /// return true if there should be a range argument before the criteria pairs
+        /// </summary>
+        protected abstract bool HasInitialRange { get; }
 
-        public interface IAggregator
+        protected interface IAggregator
         {
-            void AddValue(ValueEval d);
+            void AddValue(ValueEval value);
             ValueEval GetResult();
         }
 
-        public abstract IAggregator CreateAggregator();
+        protected abstract IAggregator CreateAggregator();
 
         public ValueEval Evaluate(ValueEval[] args, OperationEvaluationContext ec)
         {
-            bool hasInitialRange = HasInitialRange();
+            bool hasInitialRange = HasInitialRange;
             int firstCriteria = hasInitialRange ? 1 : 0;
 
-            if (args.Length < (2 + firstCriteria) || args.Length % 2 != firstCriteria)
+            if(args.Length < (2 + firstCriteria) || args.Length % 2 != firstCriteria)
             {
                 return ErrorEval.VALUE_INVALID;
             }
 
             try
             {
                 AreaEval sumRange = null;
-                if (hasInitialRange)
+                if(hasInitialRange)
                 {
-                    sumRange = convertRangeArg(args[0]);
+                    sumRange = ConvertRangeArg(args[0]);
                 }
 
                 // collect pairs of ranges and criteria
                 AreaEval[] ae = new AreaEval[(args.Length - firstCriteria) / 2];
                 IMatchPredicate[] mp = new IMatchPredicate[ae.Length];
-                for (int i = firstCriteria, k = 0; i < (args.Length - 1); i += 2, k++)
+                for(int i = firstCriteria, k = 0; i < (args.Length - 1); i += 2, k++)
                 {
-                    ae[k] = convertRangeArg(args[i]);
+                    ae[k] = ConvertRangeArg(args[i]);
 
-                    mp[k] = Countif.CreateCriteriaPredicate(args[i + 1], ec.RowIndex, ec.ColumnIndex);                    
+                    mp[k] = Countif.CreateCriteriaPredicate(args[i + 1], ec.RowIndex, ec.ColumnIndex);
                 }
 
-                validateCriteriaRanges(sumRange, ae);
-                validateCriteria(mp);
+                ValidateCriteriaRanges(sumRange, ae);
+                ValidateCriteria(mp);
 
-                return aggregateMatchingCells(CreateAggregator(), sumRange, ae, mp);
+                return AggregateMatchingCells(CreateAggregator(), sumRange, ae, mp);
             }
-            catch (EvaluationException e)
+            catch(EvaluationException e)
             {
                 return e.GetErrorEval();
             }
         }
 
-        /**
-         * Verify that each <code>criteriaRanges</code> argument contains the same number of rows and columns
-         * including the <code>sumRange</code> argument if present
-         * @param sumRange if used, it must match the shape of the criteriaRanges
-         * @param criteriaRanges to check
-         * @throws EvaluationException if the ranges do not match.
-         */
-        private static void validateCriteriaRanges(AreaEval sumRange, AreaEval[] criteriaRanges)
+        /// <summary>
+        /// Verify that each <c>criteriaRanges</c> argument contains the same number of rows and columns
+        /// including the <c>sumRange</c> argument if present
+        /// </summary>
+        /// <param name="sumRange">if used, it must match the shape of the criteriaRanges</param>
+        /// <param name="criteriaRanges">criteriaRanges to check</param>
+        /// <exception cref="EvaluationException">throws EvaluationException if the ranges do not match.</exception>
+        protected internal static void ValidateCriteriaRanges(AreaEval sumRange, AreaEval[] criteriaRanges)
         {
             int h = criteriaRanges[0].Height;
             int w = criteriaRanges[0].Width;
 
-            if (sumRange != null
+            if(sumRange != null
                     && (sumRange.Height != h
                     || sumRange.Width != w))
             {
                 throw new EvaluationException(ErrorEval.VALUE_INVALID);
 
             }
 
-            foreach (AreaEval r in criteriaRanges)
+            foreach(AreaEval r in criteriaRanges)
             {
-                if (r.Height != h ||
+                if(r.Height != h ||
                         r.Width != w)
                 {
                     throw new EvaluationException(ErrorEval.VALUE_INVALID);
                 }
             }
         }
 
-        /**
-        * Verify that each <code>criteria</code> predicate is valid, i.e. not an error
-        * @param criteria to check
-        *
-        * @throws EvaluationException if there are criteria which resulted in Errors.
-        */
-        private static void validateCriteria(IMatchPredicate[] criteria)
+        /// <summary>
+        /// Verify that each <c>criteria</c> predicate is valid, i.e. not an error
+        /// </summary>
+        /// <param name="criteria">criteria to check</param>
+        /// <exception cref="EvaluationException">throws EvaluationException if there are criteria which resulted in Errors.</exception>
+        protected internal static void ValidateCriteria(IMatchPredicate[] criteria)
         {
-            foreach (IMatchPredicate predicate in criteria)
+            foreach(IMatchPredicate predicate in criteria)
             {
                 // check for errors in predicate and return immediately using this error code
-                if (predicate is Countif.ErrorMatcher)
+                if(predicate is Countif.ErrorMatcher)
                 {
                     throw new EvaluationException(
-                        ErrorEval.ValueOf(((NPOI.SS.Formula.Functions.Countif.ErrorMatcher)predicate).Value));
+                        ErrorEval.ValueOf(((NPOI.SS.Formula.Functions.Countif.ErrorMatcher) predicate).Value));
                 }
             }
         }
@@ -115,36 +141,36 @@ private static void validateCriteria(IMatchPredicate[] criteria)
          * @return the computed value
          * @throws EvaluationException if there is an issue with eval
          */
-        private static ValueEval aggregateMatchingCells(IAggregator aggregator, AreaEval sumRange, AreaEval[] ranges, IMatchPredicate[] predicates)
+        protected static ValueEval AggregateMatchingCells(IAggregator aggregator, AreaEval sumRange, AreaEval[] ranges, IMatchPredicate[] predicates)
         {
             int height = ranges[0].Height;
             int width = ranges[0].Width;
 
-            for (int r = 0; r < height; r++) 
+            for(int r = 0; r < height; r++)
             {
-                for (int c = 0; c < width; c++) 
+                for(int c = 0; c < width; c++)
                 {
                     bool matches = true;
-                    for (int i = 0; i < ranges.Length; i++) 
+                    for(int i = 0; i < ranges.Length; i++)
                     {
                         AreaEval aeRange = ranges[i];
                         IMatchPredicate mp = predicates[i];
 
-                        if (mp == null || !mp.Matches(aeRange.GetRelativeValue(r, c))) 
+                        if(mp == null || !mp.Matches(aeRange.GetRelativeValue(r, c)))
                         {
                             matches = false;
                             break;
                         }
                     }
 
-                    if (matches)
+                    if(matches)
                     { // aggregate only if all of the corresponding criteria specified are true for that cell.
-                        if (sumRange != null)
+                        if(sumRange != null)
                         {
                             ValueEval value = sumRange.GetRelativeValue(r, c);
-                            if (value is ErrorEval) 
+                            if(value is ErrorEval)
                             {
-                                throw new EvaluationException((ErrorEval)value);
+                                throw new EvaluationException((ErrorEval) value);
                             }
                             aggregator.AddValue(value);
                         }
@@ -160,12 +186,14 @@ private static ValueEval aggregateMatchingCells(IAggregator aggregator, AreaEval
         }
 
 
-        protected static AreaEval convertRangeArg(ValueEval eval)
+        protected internal static AreaEval ConvertRangeArg(ValueEval eval)
         {
-            if (eval is AreaEval) {
+            if(eval is AreaEval)
+            {
                 return (AreaEval) eval;
             }
-            if (eval is RefEval) {
+            if(eval is RefEval)
+            {
                 return ((RefEval) eval).Offset(0, 0, 0, 0);
             }
             throw new EvaluationException(ErrorEval.VALUE_INVALID);