[github-733] Fix rate order in Mirr function. Thanks to Aleksandrs Jansons. This closes #733

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

3 files changed, 29 insertions, 35 deletions

diff --git a/poi/src/main/java/org/apache/poi/ss/formula/functions/Mirr.java b/poi/src/main/java/org/apache/poi/ss/formula/functions/Mirr.java
index d19e2d286f..ae4871ee78 100644
--- a/poi/src/main/java/org/apache/poi/ss/formula/functions/Mirr.java
+++ b/poi/src/main/java/org/apache/poi/ss/formula/functions/Mirr.java
@@ -56,23 +56,20 @@ public class Mirr extends MultiOperandNumericFunction {
     @Override
     protected double evaluate(double[] values) throws EvaluationException {
 
-        double financeRate = values[values.length-1];
-        double reinvestRate = values[values.length-2];
+        double financeRate = values[values.length-2];
+        double reinvestRate = values[values.length-1];
 
         double[] mirrValues = Arrays.copyOf(values, values.length - 2);
 
         boolean mirrValuesAreAllNegatives = true;
-        for (double mirrValue : mirrValues) {
-            mirrValuesAreAllNegatives &= mirrValue < 0;
-        }
-         if (mirrValuesAreAllNegatives) {
-             return -1.0d;
-         }
-
         boolean mirrValuesAreAllPositives = true;
         for (double mirrValue : mirrValues) {
+            mirrValuesAreAllNegatives &= mirrValue < 0;
             mirrValuesAreAllPositives &= mirrValue > 0;
         }
+        if (mirrValuesAreAllNegatives) {
+            return -1.0d;
+        }
         if (mirrValuesAreAllPositives) {
             throw new EvaluationException(ErrorEval.DIV_ZERO);
         }
@@ -88,14 +85,10 @@ public class Mirr extends MultiOperandNumericFunction {
 
         int indexN = 0;
         for (double anIn : in) {
-            if (anIn < 0) {
-                pv += anIn / Math.pow(1 + financeRate + reinvestRate, indexN++);
-            }
-        }
-
-        for (double anIn : in) {
             if (anIn > 0) {
-                fv += anIn * Math.pow(1 + financeRate, numOfYears - indexN++);
+                fv += anIn * Math.pow(1 + reinvestRate, numOfYears - indexN++);
+            } else if (anIn < 0) {
+                pv += anIn / Math.pow(1 + financeRate, indexN++);
             }
         }
 
diff --git a/poi/src/test/java/org/apache/poi/ss/formula/functions/TestMirr.java b/poi/src/test/java/org/apache/poi/ss/formula/functions/TestMirr.java
index 84843e6b44..0899e88907 100644
--- a/poi/src/test/java/org/apache/poi/ss/formula/functions/TestMirr.java
+++ b/poi/src/test/java/org/apache/poi/ss/formula/functions/TestMirr.java
@@ -44,43 +44,44 @@ final class TestMirr {
         Mirr mirr = new Mirr();
         double mirrValue;
 
-        double financeRate = 0.12;
-        double reinvestRate = 0.1;
-        double[] values = {-120000d, 39000d, 30000d, 21000d, 37000d, 46000d, reinvestRate, financeRate};
-        // MIRR should not failed with these parameters
+        double financeRate = 0.1;
+        double reinvestRate = 0.12;
+        double[] values = {-120000d, 39000d, 30000d, 21000d, 37000d, 46000d, financeRate, reinvestRate};
         mirrValue = mirr.evaluate(values);
         assertEquals(0.126094130366, mirrValue, 0.0000000001);
 
-        reinvestRate = 0.05;
-        financeRate = 0.08;
-        values = new double[]{-7500d, 3000d, 5000d, 1200d, 4000d, reinvestRate, financeRate};
-        // MIRR should not failed with these parameters
+        financeRate = 0.05;
+        reinvestRate = 0.08;
+        values = new double[]{-7500d, 3000d, 5000d, 1200d, 4000d,  financeRate, reinvestRate};
         mirrValue = mirr.evaluate(values);
         assertEquals(0.18736225093, mirrValue, 0.0000000001);
 
-        reinvestRate = 0.065;
-        financeRate = 0.1;
-        values = new double[]{-10000, 3400d, 6500d, 1000d, reinvestRate, financeRate};
-        // MIRR should not failed with these parameters
+        financeRate = 0.065;
+        reinvestRate = 0.1;
+        values = new double[]{-10000, 3400d, 6500d, 1000d,  financeRate, reinvestRate};
         mirrValue = mirr.evaluate(values);
         assertEquals(0.07039493966, mirrValue, 0.0000000001);
 
-        reinvestRate = 0.07;
-        financeRate = 0.01;
-        values = new double[]{-10000d, -3400d, -6500d, -1000d, reinvestRate, financeRate};
-        // MIRR should not failed with these parameters
+        financeRate = 0.07;
+        reinvestRate = 0.01;
+        values = new double[]{-10000d, -3400d, -6500d, -1000d, financeRate, reinvestRate};
         mirrValue = mirr.evaluate(values);
         assertEquals(-1, mirrValue, 0.0);
 
+        financeRate = 0.1;
+        reinvestRate = 0.12;
+        values = new double[]{-1000d, -4000d, 5000d, 2000d, financeRate, reinvestRate};
+        mirrValue = mirr.evaluate(values);
+        assertEquals(0.179085686035, mirrValue, 0.0000000001);
     }
 
     @Test
     void testMirrErrors_expectDIV0() {
         Mirr mirr = new Mirr();
 
-        double reinvestRate = 0.05;
         double financeRate = 0.08;
-        double[] incomes = {120000d, 39000d, 30000d, 21000d, 37000d, 46000d, reinvestRate, financeRate};
+        double reinvestRate = 0.05;
+        double[] incomes = {120000d, 39000d, 30000d, 21000d, 37000d, 46000d, financeRate, reinvestRate};
 
         EvaluationException e = assertThrows(EvaluationException.class, () -> mirr.evaluate(incomes));
         assertEquals(ErrorEval.DIV_ZERO, e.getErrorEval());
@@ -117,7 +118,7 @@ final class TestMirr {
         HSSFSheet sheet = wb.getSheet("Mirr");
         HSSFFormulaEvaluator fe = new HSSFFormulaEvaluator(wb);
         int failureCount = 0;
-        int[] resultRows = {9, 19, 29, 45};
+        int[] resultRows = {9, 19, 29, 45, 53};
 
         for (int rowNum : resultRows) {
             HSSFRow row = sheet.getRow(rowNum);
diff --git a/test-data/spreadsheet/mirrTest.xls b/test-data/spreadsheet/mirrTest.xls
index a8a0b0b12b..78ab5cfcd7 100644
--- a/test-data/spreadsheet/mirrTest.xls
+++ b/test-data/spreadsheet/mirrTest.xls