Skip to content

Commit feab3b8

Browse files
georgemac-labsbuchen
authored andcommitted
Fix dividend rate of return calculation when currencies differ
The dividend rate of return calculation was dividing the raw dividend amount (in transaction currency) by the converted moving average cost (in term currency), causing incorrect results when currencies differ. This fix uses the converted dividend amount to ensure both values are in the same currency for correct calculation. - Fix: Use converted dividend amount instead of raw amount in DividendCalculation.Payment constructor - Add test: rateOfReturnWithCurrencyMismatchBugTest to catch currency mismatch bug Fixes issue where Div%/year showed 804.61% instead of 4.99% when transaction currency (JPY) differed from term currency (EUR).
1 parent 40a0163 commit feab3b8

File tree

2 files changed

+64
-1
lines changed

2 files changed

+64
-1
lines changed

name.abuchen.portfolio.tests/src/name/abuchen/portfolio/snapshot/security/DividendCalculationTest.java

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,9 @@
1616
import name.abuchen.portfolio.model.PortfolioTransaction;
1717
import name.abuchen.portfolio.model.PortfolioTransaction.Type;
1818
import name.abuchen.portfolio.model.Security;
19+
import name.abuchen.portfolio.model.SecurityPrice;
1920
import name.abuchen.portfolio.money.CurrencyConverter;
21+
import name.abuchen.portfolio.money.Values;
2022
import name.abuchen.portfolio.snapshot.security.BaseSecurityPerformanceRecord.Periodicity;
2123

2224
@SuppressWarnings("nls")
@@ -196,4 +198,61 @@ public void rateOfReturnCalculationTest()
196198

197199
assertEquals(0.1, dividends.getRateOfReturnPerYear(), 0.0);
198200
}
201+
202+
@Test
203+
public void rateOfReturnWithCurrencyMismatchBugTest()
204+
{
205+
// Test case that catches the currency mismatch bug:
206+
// When transaction currency (USD) differs from term currency (EUR),
207+
// the bug divides raw USD amount by converted EUR cost without proper conversion.
208+
//
209+
// Purchase: 10 shares at 100 USD per share = 1000 USD total
210+
// Dividend: 50 USD (5 USD per share) on 2015-01-15
211+
//
212+
// Using TestCurrencyConverter with EUR term currency:
213+
// Exchange rate on 2015-01-15: ~1.1708 EUR per USD (from test data)
214+
// - Purchase cost in EUR: 1000 USD * 1.1708 = 1170.8 EUR
215+
// - Dividend in EUR: 50 USD * 1.1708 = 58.54 EUR
216+
// - Correct rate: 58.54 / 1170.8 = 0.05 (5%)
217+
//
218+
// BUG: Code uses raw dividend amount (5000 in stored format, USD) divided by
219+
// converted cost (in EUR), which gives wrong result
220+
// With buggy code: ~0.0588 (wrong - dividing USD by EUR)
221+
// With fixed code: ~0.05 (correct - both in EUR)
222+
223+
Security testSecurity = new Security("Test Security", "USD");
224+
testSecurity.addPrice(new SecurityPrice(LocalDateTime.of(2015, 1, 15, 0, 0).toLocalDate(),
225+
Values.Quote.factorize(100)));
226+
227+
List<CalculationLineItem> transactions = new ArrayList<>();
228+
229+
// Purchase: 10 shares at 100 USD = 1000 USD
230+
transactions.add(CalculationLineItem.of(new Portfolio(), new PortfolioTransaction(
231+
LocalDateTime.of(2015, 1, 14, 12, 0), testSecurity.getCurrencyCode(),
232+
Values.Amount.factorize(1000), testSecurity, Values.Share.factorize(10), Type.BUY, 0L, 0L)));
233+
234+
// Dividend: 50 USD (5 USD per share)
235+
AccountTransaction dividend = new AccountTransaction();
236+
dividend.setType(AccountTransaction.Type.DIVIDENDS);
237+
dividend.setSecurity(testSecurity);
238+
dividend.setDateTime(LocalDateTime.of(2015, 1, 15, 12, 0));
239+
dividend.setAmount(Values.Amount.factorize(50));
240+
dividend.setShares(Values.Share.factorize(10));
241+
dividend.setCurrencyCode(testSecurity.getCurrencyCode());
242+
243+
transactions.add(CalculationLineItem.of(new Account(), dividend));
244+
245+
@SuppressWarnings("unused")
246+
CostCalculation cost = Calculation.perform(CostCalculation.class, converter, testSecurity, transactions);
247+
DividendCalculation dividends = Calculation.perform(DividendCalculation.class, converter, testSecurity,
248+
transactions);
249+
250+
// Expected: 50 USD / 1000 USD = 0.05 (5%)
251+
// After currency conversion to EUR: ~58.54 EUR / ~1170.8 EUR = 0.05
252+
// With bug: Uses raw USD amount / converted EUR cost = ~0.0588 (wrong!)
253+
// With fix: Uses converted dividend / converted cost = ~0.05 (correct)
254+
// This test will FAIL with the buggy code (~0.0588) and PASS with the fix (~0.05)
255+
// Tolerance is tight (0.005) to catch the bug: 0.0588 - 0.05 = 0.0088 > 0.005
256+
assertEquals(0.05, dividends.getRateOfReturnPerYear(), 0.005);
257+
}
199258
}

name.abuchen.portfolio/src/name/abuchen/portfolio/snapshot/security/DividendCalculation.java

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,11 @@ public Payment(CurrencyConverter converter, CalculationLineItem.DividendPayment
7474

7575
Money movingAverageCost = t.getMovingAverageCost();
7676
if (movingAverageCost != null && !movingAverageCost.isZero())
77-
rr = t.getGrossValueAmount() / (double) movingAverageCost.getAmount();
77+
{
78+
// Use converted amount (in term currency) instead of raw amount (in transaction currency)
79+
// to ensure both values are in the same currency for correct calculation
80+
rr = this.amount.getAmount() / (double) movingAverageCost.getAmount();
81+
}
7882

7983
// check if it is valid (non 0)
8084
if (rr == 0)

0 commit comments

Comments
 (0)