From edb9aeaca502b7799500a5ec994c5bf7f4e37fcf Mon Sep 17 00:00:00 2001 From: PJ Fanning Date: Mon, 25 Nov 2024 20:48:45 +0000 Subject: [PATCH] [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 --- .../apache/poi/ss/formula/functions/Mirr.java | 25 ++++------- .../poi/ss/formula/functions/TestMirr.java | 39 +++++++++--------- test-data/spreadsheet/mirrTest.xls | Bin 28160 -> 29184 bytes 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); } @@ -87,15 +84,11 @@ public class Mirr extends MultiOperandNumericFunction { double fv = 0; 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 a8a0b0b12b89bad42b04b11c78eacb20e875b34e..78ab5cfcd78bb03a54cc82be28465217c3befb2f 100644 GIT binary patch delta 3130 zcma)8TWl0%6h8m#>}__tyOgCjy2G;D0$sZ8ZrNU-mTqZFVL|Z13#Hg%SVFNB)5?RU zaWH{sBwWg`h9|(pDr&h7q6kQN(SV_9eDDE70`bY9KIjt>>NzvZUI*eN^Y1y|`OoD) z|2b!N&#!FHZ|rb{HIGNJhzsQY=bQ( zVK%S;O!@;5gj#%q5P??AX&CZRS!@Iglo@T`1TR1mw!-E@MFs82Vl!Bwyil_ag->#) z%Ve(K353*Stlku`_x9#T-|_XN!Oz_4z$W*E!4n5 zfc|jEAFK@qBa!}aYu{rAX%Ym&0ga0Hhh=d$roAOAGTtH0aqD)|w?LSZ@ulAk#i&Qo z4(*!vR&9M8NemW4eO4rOxv}VMDxCKAOhA?J5f(87ssh>p`fFPI79r!*xV{=$oDUPc z(^ADg=G!gR>=S;_5@IL$CCf7QDgVb3WuNf{))nk?K4@KPo$+V5S*(CB_@w0XT?99Z*AJsqU$$*S>1Oy6 zlh1gC!_O^_XOyo%1c=l{s%6mum$2B0i4g0Aud#x4miQvLjAfbP32?y`u%`h++;A17 z3__3xz5z!TMA$62hOMqyDneiMAX*Gl>a=5?$W;EPyA)4O>v?TGt2TMQW9%A#G;62W z>li3icNISfR*kpdEW-Go9UK|i2QFB;ig*jWxILMCt$~nhtNDSZwf4r}UGOisSiO3$ zX*QGQHUs&GiDo|2ynR9TpmhjkWJ7ys8o%BK5bgvx)x!_8JjVj+otC#nWq-&`nF0`8 z>7B?RZ4>ZL@DU(DV-$e7HB6tWxft!w-)Azw@PcypDd@zU4H^3?k zu(<_T)Pp@;XwYYX&C@aUNP9UmMc(~UJsp2vP;%>FzJXrA01FymRR&nK0fyr_lK>@E zl#8;#6joz^g$%I823V~D7B;{X9MPF*QjQ}ASkwS(Fu;}=U@-%%QODF+*D=Os_|M&S z%A0}&6b_ba{Ls`x9AEXokfY`#8HGi%Gnx!Pr^r#0l8nMb**j{S^rD)Knw4Y}Cg~m5 zWDS~(dX{7qZYLsD@rMd3F42TIBsC$$O~OT*VoZ}!b6Ow<6n__-ytVOI&;4)PaWVls zlA*euyiLJWc^p_58rKOlG}X}t6k4NZ1t&B7g2R6W#yG|*v4$&QGc7AMk|=);N62!a z7%p0|qG)W-vzXs><%7m9);L${|d<>1# zM0*cknqnvLrJ*v(cP18=dXY-~hpzQ!{#WzkiIAnd`0o$eZmT~es@)mWCEVBh7W1gb zd;f5EQyWs-J(`)Bp;;7h}x!d79xlZ3m@&G2Z5$-21!d zo^yN8IrokIj$=RJL=1OK#y6?k0pRT3Ge|4wY|oZp znCFwT&-@e73-+wZ5WSxp1?aq8j9;?nwnX&REhTt2JOM*62v5QY%PO@J?DU`(K?fbz zYO#dg(dtm6ueD}ura!d~Y@&K+99!sNXE$!4cbz+JA3>13dWfz$EAeA;>s|N>?bf^T zQ<~5dI72_{$80BI8~w^UL#};ziY~fdb<9F1-~l@7uA`KDU&S040M(BSj8=E%Wb)bS zDzySWqhH+N@M-Yz^8H76H7gDs;?>9C4731J!V{%-&mHM`Pyu4CvAQbN1!pDlJc-HF z4GU0`o66h+CTO?j0=)1!I9GuXAAAA&8WCi`IdJ76VH;qP2pSN!V38~{zu8reHb)f9-@4yY(s{Au z5N)>DS8c&+ELg1ttFvGY`7E7F%Eo+{V@(#U*@A7cU@aExwgUE&qJz28JBfIcChu)+ zx(rqDEmy*IzS3L2-qlf*M|UZHIOlNE^+cmLD31^ze;|0I(R6=9)%xnqRv+tR+T=ai zs-^k+_VN=vo(NN>5vK1#E*QQcKCG%64>Q7%; bqS4d|<@G+gk$N6$>G;vKTVf=i4r%`aT$e8%