From 41c3f77eb30037df195ed935170c8d94c10fe36b Mon Sep 17 00:00:00 2001 From: mierin12 Date: Fri, 2 Jan 2026 22:42:08 +0100 Subject: [PATCH 1/2] fix moving average entry price in Trades when security is shared between several portfolio --- .../snapshot/trades/TradeCollector6Test.java | 114 +++++++++++++++++- .../portfolio/snapshot/trades/TradeTest.java | 20 --- .../filter/ClientTransactionFilter.java | 82 ------------- .../portfolio/snapshot/trades/Trade.java | 80 ++++++------ .../snapshot/trades/TradeCollector.java | 13 +- 5 files changed, 159 insertions(+), 150 deletions(-) delete mode 100644 name.abuchen.portfolio/src/name/abuchen/portfolio/snapshot/filter/ClientTransactionFilter.java diff --git a/name.abuchen.portfolio.tests/src/name/abuchen/portfolio/snapshot/trades/TradeCollector6Test.java b/name.abuchen.portfolio.tests/src/name/abuchen/portfolio/snapshot/trades/TradeCollector6Test.java index 83060b64e5..faf46fe892 100644 --- a/name.abuchen.portfolio.tests/src/name/abuchen/portfolio/snapshot/trades/TradeCollector6Test.java +++ b/name.abuchen.portfolio.tests/src/name/abuchen/portfolio/snapshot/trades/TradeCollector6Test.java @@ -1,10 +1,13 @@ package name.abuchen.portfolio.snapshot.trades; +import static name.abuchen.portfolio.junit.PortfolioBuilder.amountOf; import static org.hamcrest.CoreMatchers.is; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.closeTo; +import java.time.LocalDate; import java.time.LocalDateTime; +import java.util.Collections; import java.util.List; import org.junit.Test; @@ -18,6 +21,9 @@ import name.abuchen.portfolio.money.CurrencyUnit; import name.abuchen.portfolio.money.Money; import name.abuchen.portfolio.money.Values; +import name.abuchen.portfolio.snapshot.filter.PortfolioClientFilter; +import name.abuchen.portfolio.snapshot.security.LazySecurityPerformanceSnapshot; +import name.abuchen.portfolio.util.Interval; @SuppressWarnings("nls") public class TradeCollector6Test @@ -58,7 +64,7 @@ public void testTradesSimpleOpenPosition() throws TradeCollectorException } @Test - public void testTradesProtitAndGrossProfitFIFOAndMovingAverage() throws TradeCollectorException + public void testTradesProfitAndGrossProfitFIFOAndMovingAverage() throws TradeCollectorException { Client client = new Client(); @@ -123,4 +129,110 @@ public void testTradesProtitAndGrossProfitFIFOAndMovingAverage() throws TradeCol assertThat(secondTrade.getReturnMovingAverage(), closeTo(0.9737, 0.0001)); } + @Test + public void testMovingAverageTradesSeveralPortfolio() throws TradeCollectorException + { + Client client = new Client(); + + Security security = new SecurityBuilder().addTo(client); + new PortfolioBuilder(new Account("one")).buyPrice(security, "2022-01-01", 5, 100).addTo(client); + new PortfolioBuilder(new Account("two")).buyPrice(security, "2022-02-02", 5, 200).addTo(client); + + TradeCollector collector = new TradeCollector(client, new TestCurrencyConverter()); + List trades = collector.collect(security); + Collections.sort(trades, (r, l) -> r.getStart().compareTo(l.getStart())); + + assertThat(trades.size(), is(2)); + + Trade firstTrade = trades.get(0); + + assertThat(firstTrade.getStart(), is(LocalDateTime.parse("2022-01-01T00:00"))); + assertThat(firstTrade.getEntryValueMovingAverage(), is(Money.of(CurrencyUnit.EUR, amountOf(100 * 5)))); + + Trade secondTrade = trades.get(1); + + assertThat(secondTrade.getStart(), is(LocalDateTime.parse("2022-02-02T00:00"))); + assertThat(secondTrade.getEntryValueMovingAverage(), is(Money.of(CurrencyUnit.EUR, amountOf(200 * 5)))); + } + + @Test + public void testMovingAverageTradesSeveralPortfolio2() throws TradeCollectorException + { + Client client = new Client(); + + Security security = new SecurityBuilder().addPrice("2023-03-01", Values.Quote.factorize(200)) // + .addTo(client); + var p1 = new PortfolioBuilder(new Account("one")).buyPrice(security, "2022-01-01", 5, 100, 10, 10) + .buyPrice(security, "2022-02-01", 10, 100) + .sellPrice(security, "2022-03-01", 10, 150, 10, 10) + .addTo(client); + + var p2 = new PortfolioBuilder(new Account("two")).buyPrice(security, "2022-01-02", 5, 200, 10, 10) + .buyPrice(security, "2022-02-02", 10, 200) + .sellPrice(security, "2022-03-02", 10, 150, 10, 10) + .addTo(client); + + TradeCollector collector = new TradeCollector(client, new TestCurrencyConverter()); + List trades = collector.collect(security); + Collections.sort(trades, (r, l) -> r.getStart().compareTo(l.getStart())); + + assertThat(trades.size(), is(4)); + + Trade firstClosedTrade = trades.get(0); + + assertThat(firstClosedTrade.getStart(), is(LocalDateTime.parse("2022-01-01T00:00"))); + assertThat(firstClosedTrade.getEnd().isPresent(), is(true)); + // 1500 - (5*100 + 10*100) * 10/15 = 500 + assertThat(firstClosedTrade.getProfitLossMovingAverage(), + is(Money.of(CurrencyUnit.EUR, Values.Amount.factorize(500)))); + // 1500+10+10 - (500-10-10 + 1000) * 10/15 = 533.33 + assertThat(firstClosedTrade.getProfitLossMovingAverageWithoutTaxesAndFees(), + is(Money.of(CurrencyUnit.EUR, Values.Amount.factorize(533.33)))); + + Trade secondClosedTrade = trades.get(1); + + assertThat(secondClosedTrade.getStart(), is(LocalDateTime.parse("2022-01-02T00:00"))); + assertThat(secondClosedTrade.getEnd().isPresent(), is(true)); + // 1500 - (5*200 + 10*200) * 10/15 = -500 + assertThat(secondClosedTrade.getProfitLossMovingAverage(), + is(Money.of(CurrencyUnit.EUR, Values.Amount.factorize(-500)))); + // 1500+10+10 - (1000-10-10 + 2000) * 10/15 = -466.666 + assertThat(secondClosedTrade.getProfitLossMovingAverageWithoutTaxesAndFees(), + is(Money.of(CurrencyUnit.EUR, Values.Amount.factorize(-466.67)))); + + Trade firstOpenTrade = trades.get(2); + + assertThat(firstOpenTrade.getEnd().isPresent(), is(false)); + assertThat(firstOpenTrade.getStart(), is(LocalDateTime.parse("2022-02-01T00:00"))); + // 200 * 5 - (500 + 1000) * 5/15 = 500 + assertThat(firstOpenTrade.getProfitLossMovingAverage(), + is(Money.of(CurrencyUnit.EUR, Values.Amount.factorize(500)))); + // 200 * 5 - (500-10-10 + 1000) * 5/15 = 506.666 + assertThat(firstOpenTrade.getProfitLossMovingAverageWithoutTaxesAndFees(), + is(Money.of(CurrencyUnit.EUR, Values.Amount.factorize(506.67)))); + + var snapshot = LazySecurityPerformanceSnapshot.create(new PortfolioClientFilter(p1).filter(client), + new TestCurrencyConverter(), Interval.of(LocalDate.MIN, LocalDate.now())); + var securityRecord = snapshot.getRecord(security).get(); + + assertThat(firstOpenTrade.getEntryValueMovingAverage(), is(securityRecord.getMovingAverageCost().get())); + + Trade secondOpenTrade = trades.get(3); + + assertThat(secondOpenTrade.getEnd().isPresent(), is(false)); + assertThat(secondOpenTrade.getStart(), is(LocalDateTime.parse("2022-02-02T00:00"))); + // 200 * 5 - (1000 + 2000) * 5/15 = 0 + assertThat(secondOpenTrade.getProfitLossMovingAverage(), + is(Money.of(CurrencyUnit.EUR, Values.Amount.factorize(0.00)))); + // 200 * 5 - (1000-10-10 + 2000) * 5/15 = 6.666 + assertThat(secondOpenTrade.getProfitLossMovingAverageWithoutTaxesAndFees(), + is(Money.of(CurrencyUnit.EUR, Values.Amount.factorize(6.67)))); + + snapshot = LazySecurityPerformanceSnapshot.create(new PortfolioClientFilter(p2).filter(client), + new TestCurrencyConverter(), Interval.of(LocalDate.MIN, LocalDate.now())); + securityRecord = snapshot.getRecord(security).get(); + + assertThat(secondOpenTrade.getEntryValueMovingAverage(), is(securityRecord.getMovingAverageCost().get())); + } + } diff --git a/name.abuchen.portfolio.tests/src/name/abuchen/portfolio/snapshot/trades/TradeTest.java b/name.abuchen.portfolio.tests/src/name/abuchen/portfolio/snapshot/trades/TradeTest.java index db98c3eb76..7c359bee84 100644 --- a/name.abuchen.portfolio.tests/src/name/abuchen/portfolio/snapshot/trades/TradeTest.java +++ b/name.abuchen.portfolio.tests/src/name/abuchen/portfolio/snapshot/trades/TradeTest.java @@ -126,26 +126,6 @@ public void testShort() throws TradeCollectorException assertEquals(trade1.getIRR(), 0.75, 0.0001); } - @Test - public void testShortMovingAverageCostDoesNotDivideByZero() throws TradeCollectorException - { - Client client = new Client(); - TradeCollector collector = new TradeCollector(client, new TestCurrencyConverter()); - - var portfolio = new PortfolioBuilder(); - portfolio.addTo(client); - - Security securityShort = new SecurityBuilder().addTo(client); - portfolio.sellPrice(securityShort, "2024-01-01", 3.0, 20.0).buyPrice(securityShort, "2024-12-31", 3.0, 5.0); - - List trades = collector.collect(securityShort); - Trade trade = trades.get(0); - - Money movingAverageEntryValue = trade.getEntryValueMovingAverage(); - - assertThat(movingAverageEntryValue, is(Money.of(CurrencyUnit.EUR, 0L))); - } - @Test public void testLongMultipleBuys() throws TradeCollectorException { diff --git a/name.abuchen.portfolio/src/name/abuchen/portfolio/snapshot/filter/ClientTransactionFilter.java b/name.abuchen.portfolio/src/name/abuchen/portfolio/snapshot/filter/ClientTransactionFilter.java deleted file mode 100644 index 917c6b6f88..0000000000 --- a/name.abuchen.portfolio/src/name/abuchen/portfolio/snapshot/filter/ClientTransactionFilter.java +++ /dev/null @@ -1,82 +0,0 @@ -package name.abuchen.portfolio.snapshot.filter; - -import java.util.Collections; - -import name.abuchen.portfolio.model.Account; -import name.abuchen.portfolio.model.Client; -import name.abuchen.portfolio.model.Portfolio; -import name.abuchen.portfolio.model.PortfolioTransaction; -import name.abuchen.portfolio.model.Security; -import name.abuchen.portfolio.snapshot.trades.TradeCollector; - -/** - * Remove all transactions starting with the given transaction. Transactions are - * sorted by exactly how the TradeCollector is sorting the transactions. It is - * used to create a client that allows to calculate the moving average costs for - * a given trade, i.e., the costs before the sale is applied.
- *
- * Attention: This filter processes only portfolio transactions. - */ -public class ClientTransactionFilter implements ClientFilter -{ - private final Security security; - private final PortfolioTransaction transaction; - - public ClientTransactionFilter(Security security, PortfolioTransaction transaction) - { - this.security = security; - this.transaction = transaction; - } - - @Override - public Client filter(Client client) - { - var txs = security.getTransactions(client); - - Collections.sort(txs, new TradeCollector.ByDateAndType()); - - // find the transactions in the list - var index = -1; - for (int ii = 0; ii < txs.size(); ii++) - { - if (txs.get(ii).getTransaction().equals(transaction)) - { - index = ii; - break; - } - } - - // limit the transactions to all transactions before the given one - if (index > 0) - txs = txs.subList(0, index); - - ReadOnlyClient pseudoClient = new ReadOnlyClient(client); - pseudoClient.internalAddSecurity(security); - - Account account = new Account(); - Portfolio portfolio = new Portfolio(); - portfolio.setReferenceAccount(account); - - ReadOnlyAccount pa = new ReadOnlyAccount(account); - pseudoClient.internalAddAccount(pa); - - ReadOnlyPortfolio pp = new ReadOnlyPortfolio(portfolio); - pp.setReferenceAccount(pa); - pseudoClient.internalAddPortfolio(pp); - - for (var tx : txs) - { - var t = tx.getTransaction(); - - if (t instanceof PortfolioTransaction tp // - && tp.getType() != PortfolioTransaction.Type.TRANSFER_IN - && tp.getType() != PortfolioTransaction.Type.TRANSFER_OUT) - { - pp.internalAddTransaction(tp); - } - } - - return pseudoClient; - } - -} diff --git a/name.abuchen.portfolio/src/name/abuchen/portfolio/snapshot/trades/Trade.java b/name.abuchen.portfolio/src/name/abuchen/portfolio/snapshot/trades/Trade.java index db6f530eef..da6668231f 100644 --- a/name.abuchen.portfolio/src/name/abuchen/portfolio/snapshot/trades/Trade.java +++ b/name.abuchen.portfolio/src/name/abuchen/portfolio/snapshot/trades/Trade.java @@ -13,7 +13,6 @@ import name.abuchen.portfolio.math.IRR; import name.abuchen.portfolio.model.Adaptable; -import name.abuchen.portfolio.model.Client; import name.abuchen.portfolio.model.Named; import name.abuchen.portfolio.model.Portfolio; import name.abuchen.portfolio.model.PortfolioTransaction; @@ -24,11 +23,8 @@ import name.abuchen.portfolio.money.Money; import name.abuchen.portfolio.money.MoneyCollectors; import name.abuchen.portfolio.money.Values; -import name.abuchen.portfolio.snapshot.filter.ClientTransactionFilter; import name.abuchen.portfolio.snapshot.security.LazySecurityPerformanceRecord.LazyValue; -import name.abuchen.portfolio.snapshot.security.LazySecurityPerformanceSnapshot; import name.abuchen.portfolio.util.Dates; -import name.abuchen.portfolio.util.Interval; public class Trade implements Adaptable { @@ -51,6 +47,7 @@ private CollateralLot(long shares, double amount) private final long shares; private List> transactions = new ArrayList<>(); + private List> transactionsMovingAverage = new ArrayList<>(); private Money entryValue; private Money entryValueWithoutTaxesAndFees; @@ -74,7 +71,7 @@ public boolean isLong() return transactions.get(0).getTransaction().getType().isPurchase(); } - /* package */ void calculate(Client client, CurrencyConverter converter) + /* package */ void calculate(CurrencyConverter converter) { boolean isLong = this.isLong(); @@ -148,9 +145,9 @@ public boolean isLong() calculateIRR(converter); this.entryValueMovingAverage = new LazyValue<>( - () -> getMovingAverageCost(client, converter, TaxesAndFees.INCLUDED)); + () -> getMovingAverageCost(converter, TaxesAndFees.INCLUDED)); this.entryValueMovingAverageWithoutTaxesAndFees = new LazyValue<>( - () -> getMovingAverageCost(client, converter, TaxesAndFees.NOT_INCLUDED)); + () -> getMovingAverageCost(converter, TaxesAndFees.NOT_INCLUDED)); } private void calculateIRR(CurrencyConverter converter) @@ -290,6 +287,11 @@ public List> getTransactions() return transactions; } + public List> getTransactionsMovingAverage() + { + return transactionsMovingAverage; + } + public TransactionPair getLastTransaction() { // transactions have been sorted by calculate(), which is called once @@ -412,47 +414,37 @@ public String toString() shares, start, entryValue, end, exitValue); } - private Money getMovingAverageCost(Client client, CurrencyConverter converter, TaxesAndFees taxesAndFees) + private Money getMovingAverageCost(CurrencyConverter converter, TaxesAndFees taxesAndFees) { - var closingTransaction = transactions.stream() // - .filter(t -> t.getTransaction().getType().isLiquidation()) // - .findFirst().map(t -> t.getTransaction()); + var holdShares = 0d; + var amount = 0L; - Client filteredClient = client; - if (closingTransaction.isPresent()) + Collections.sort(transactionsMovingAverage, TradeCollector.BY_DATE_AND_TYPE); + for (TransactionPair t : transactionsMovingAverage) { - // if a closing transaction is present, we need to calculate the - // moving average costs based on all transactions before the - // closing transaction - - filteredClient = new ClientTransactionFilter(security, closingTransaction.get()).filter(client); - } - - var snapshot = LazySecurityPerformanceSnapshot.create(filteredClient, converter, - Interval.of(LocalDate.MIN, - closingTransaction.isPresent() - ? closingTransaction.get().getDateTime().toLocalDate() - : LocalDate.now())); - var r = snapshot.getRecord(security); - if (r.isEmpty()) - return null; - - // the trade might be a partial liquidation, so we have to calculate - // the moving average purchase value based on the number of shares - // sold - - var totalCosts = taxesAndFees == TaxesAndFees.INCLUDED // - ? r.get().getMovingAverageCost().get() - : r.get().getMovingAverageCostWithoutTaxesAndFees().get(); - var totalShares = r.get().getSharesHeld().get(); - - if (totalShares <= 0) - return Money.of(totalCosts.getCurrencyCode(), 0); + PortfolioTransaction tx = t.getTransaction(); + if (tx.getType().isPurchase()) + { + holdShares += tx.getShares(); + amount += taxesAndFees == TaxesAndFees.INCLUDED + ? tx.getMonetaryAmount().with(converter.at(tx.getDateTime())).getAmount() + : tx.getGrossValue().with(converter.at(tx.getDateTime())).getAmount(); + } + else + { + if (holdShares == 0) + return Money.of(converter.getTermCurrency(), 0); - var cost = BigDecimal.valueOf(shares / (double) totalShares) // - .multiply(BigDecimal.valueOf(totalCosts.getAmount())) // - .setScale(0, RoundingMode.HALF_DOWN).longValue(); + boolean isClosingTransaction = getClosingTransaction().isPresent() + && getClosingTransaction().get().getTransaction().equals(tx); + var shareRatio = BigDecimal.valueOf( + (isClosingTransaction ? tx.getShares() : holdShares - tx.getShares()) / holdShares); - return Money.of(totalCosts.getCurrencyCode(), cost); + holdShares -= tx.getShares(); + amount = shareRatio.multiply(BigDecimal.valueOf(amount)).setScale(0, RoundingMode.HALF_DOWN) + .longValue(); + } + } + return Money.of(converter.getTermCurrency(), amount); } } diff --git a/name.abuchen.portfolio/src/name/abuchen/portfolio/snapshot/trades/TradeCollector.java b/name.abuchen.portfolio/src/name/abuchen/portfolio/snapshot/trades/TradeCollector.java index 460ca0b4ba..6cad8aae8a 100644 --- a/name.abuchen.portfolio/src/name/abuchen/portfolio/snapshot/trades/TradeCollector.java +++ b/name.abuchen.portfolio/src/name/abuchen/portfolio/snapshot/trades/TradeCollector.java @@ -108,6 +108,7 @@ public List collect(Security security) throws TradeCollectorException List trades = new ArrayList<>(); Map>> openTransactions = new HashMap<>(); + Map>> allTransactions = new HashMap<>(); for (TransactionPair txp : transactions) { @@ -131,14 +132,16 @@ public List collect(Security security) throws TradeCollectorException // a new trade. (Note: it's an invariant that fifo contains // transaction of the same type (purchase vs !purchase), so // we test 0th element in fifo). + allTransactions.computeIfAbsent(portfolio, p -> new ArrayList<>()).add(pair); if (openList.isEmpty() || openList.get(0).getTransaction().getType().isPurchase() == type.isPurchase()) openList.add(pair); else - trades.add(createNewTrade(openTransactions, pair)); + trades.add(createNewTrade(openTransactions, pair, allTransactions)); break; case TRANSFER_IN: moveOpenTransaction(openTransactions, pair); + moveOpenTransaction(allTransactions, pair); break; case TRANSFER_OUT: @@ -165,17 +168,20 @@ public List collect(Security security) throws TradeCollectorException Trade newTrade = new Trade(security, entry.getKey(), shares); newTrade.setStart(position.get(0).getTransaction().getDateTime()); newTrade.getTransactions().addAll(position); + newTrade.getTransactionsMovingAverage().addAll(allTransactions.get(entry.getKey())); trades.add(newTrade); } - trades.forEach(t -> t.calculate(client, converter)); + trades.forEach(t -> t.calculate(converter)); return trades; } private Trade createNewTrade(Map>> openTransactions, - TransactionPair pair) throws TradeCollectorException + TransactionPair pair, + Map>> allTransactions) + throws TradeCollectorException { Trade newTrade = new Trade(pair.getTransaction().getSecurity(), (Portfolio) pair.getOwner(), pair.getTransaction().getShares()); @@ -225,6 +231,7 @@ else if (sharesToDistribute < candidate.getTransaction().getShares()) pair.getOwner(), Values.Share.format(sharesToDistribute), pair)); } + newTrade.getTransactionsMovingAverage().addAll(allTransactions.get(pair.getOwner())); newTrade.getTransactions().add(pair); newTrade.setEnd(pair.getTransaction().getDateTime()); From c043009bd00a94b32093b871d6eff57a4e5bfd79 Mon Sep 17 00:00:00 2001 From: mierin12 Date: Sun, 18 Jan 2026 22:41:30 +0100 Subject: [PATCH 2/2] add support for Short/Long moving average trades --- .../portfolio/snapshot/trades/TradeTest.java | 54 +++++++++++++++++++ .../portfolio/snapshot/trades/Trade.java | 24 ++++++--- 2 files changed, 71 insertions(+), 7 deletions(-) diff --git a/name.abuchen.portfolio.tests/src/name/abuchen/portfolio/snapshot/trades/TradeTest.java b/name.abuchen.portfolio.tests/src/name/abuchen/portfolio/snapshot/trades/TradeTest.java index 7c359bee84..f41213b070 100644 --- a/name.abuchen.portfolio.tests/src/name/abuchen/portfolio/snapshot/trades/TradeTest.java +++ b/name.abuchen.portfolio.tests/src/name/abuchen/portfolio/snapshot/trades/TradeTest.java @@ -5,6 +5,7 @@ import static name.abuchen.portfolio.junit.PortfolioBuilder.sharesOf; import static org.hamcrest.CoreMatchers.is; import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.closeTo; import static org.junit.Assert.assertEquals; import java.time.LocalDateTime; @@ -19,6 +20,7 @@ import name.abuchen.portfolio.model.Security; import name.abuchen.portfolio.money.CurrencyUnit; import name.abuchen.portfolio.money.Money; +import name.abuchen.portfolio.money.Values; /** * This is intended to be unit test for Trade (and by extension, TradeCollector) @@ -55,6 +57,10 @@ public void testLong() throws TradeCollectorException assertThat(trade1.getProfitLoss(), is(Money.of(CurrencyUnit.EUR, amountOf(180 - 100) * 5))); assertThat(trade1.getReturn(), is(0.8)); assertEquals(trade1.getIRR(), 0.8, 0.0001); + + assertThat(trade1.getEntryValueMovingAverage(), is(trade1.getEntryValue())); + assertThat(trade1.getProfitLossMovingAverage(), is(trade1.getProfitLoss())); + assertThat(trade1.getReturnMovingAverage(), is(trade1.getReturn())); } @Test @@ -124,6 +130,10 @@ public void testShort() throws TradeCollectorException assertThat(trade1.getProfitLoss(), is(Money.of(CurrencyUnit.EUR, amountOf(20 - 5) * 3))); assertThat(trade1.getReturn(), is(0.75)); assertEquals(trade1.getIRR(), 0.75, 0.0001); + + assertThat(trade1.getEntryValueMovingAverage(), is(trade1.getEntryValue())); + assertThat(trade1.getProfitLossMovingAverage(), is(trade1.getProfitLoss())); + assertThat(trade1.getReturnMovingAverage(), is(trade1.getReturn())); } @Test @@ -199,4 +209,48 @@ public void testShortMultipleSells() throws TradeCollectorException assertThat(trade2.getProfitLoss(), is(Money.of(CurrencyUnit.EUR, amountOf(1 * 120 + 2 * 50 - 0)))); assertThat(trade2.getReturn(), is(1.0)); } + + @Test + public void testShortMultipleSellsMovingAverage() throws TradeCollectorException + { + Client client = new Client(); + TradeCollector collector = new TradeCollector(client, new TestCurrencyConverter()); + List trades; + + var port = new PortfolioBuilder(); + port.addTo(client); + + Security securityShort = new SecurityBuilder().addPrice("2025-03-01", Values.Quote.factorize(30)).addTo(client); + port.sellPrice(securityShort, "2024-01-01", 2.0, 100.0).sellPrice(securityShort, "2024-02-01", 3.0, 120.0) + .sellPrice(securityShort, "2024-03-01", 2.0, 50.0) + .buyPrice(securityShort, "2024-12-31", 4.0, 20.0); + + trades = collector.collect(securityShort); + assertThat(trades.size(), is(2)); + + Trade trade1 = trades.get(0); + assertThat(trade1.isClosed(), is(true)); + assertThat(trade1.isLong(), is(false)); + assertThat(trade1.getShares(), is(sharesOf(4))); + // entryAmount = (2 * 100 + 3 * 120 + 2 * 50) * 4 / 7 = 377.142857 + var exitAmount = 4 * 20; + assertThat(trade1.getEntryValueMovingAverage(), + is(Money.of(CurrencyUnit.EUR, Values.Amount.factorize(377.14)))); + assertThat(trade1.getExitValue(), is(Money.of(CurrencyUnit.EUR, amountOf(exitAmount)))); + assertThat(trade1.getProfitLossMovingAverage(), + is(Money.of(CurrencyUnit.EUR, Values.Amount.factorize(377.14 - exitAmount)))); + assertThat(trade1.getReturnMovingAverage(), closeTo(1.0 - (double) exitAmount / 377.14, 0.0001)); + assertEquals(0.8710, trade1.getIRR(), 0.0001); + + Trade trade2 = trades.get(1); + // entryAmount2 = (2 * 100 + 3 * 120 + 2 * 50) * 3 / 7 = 282.85714 + var exitAmount2 = 3 * 30; + + assertThat(trade2.isClosed(), is(false)); + assertThat(trade2.isLong(), is(false)); + assertThat(trade2.getShares(), is(sharesOf(3))); + assertThat(trade2.getEnd().isPresent(), is(false)); + assertThat(trade2.getProfitLossMovingAverage(), is(Money.of(CurrencyUnit.EUR, amountOf(282.86 - exitAmount2)))); + assertThat(trade2.getReturnMovingAverage(), is(1 - exitAmount2 / 282.86)); + } } diff --git a/name.abuchen.portfolio/src/name/abuchen/portfolio/snapshot/trades/Trade.java b/name.abuchen.portfolio/src/name/abuchen/portfolio/snapshot/trades/Trade.java index da6668231f..0e7855c364 100644 --- a/name.abuchen.portfolio/src/name/abuchen/portfolio/snapshot/trades/Trade.java +++ b/name.abuchen.portfolio/src/name/abuchen/portfolio/snapshot/trades/Trade.java @@ -145,9 +145,9 @@ public boolean isLong() calculateIRR(converter); this.entryValueMovingAverage = new LazyValue<>( - () -> getMovingAverageCost(converter, TaxesAndFees.INCLUDED)); + () -> getMovingAverageCost(converter, TaxesAndFees.INCLUDED, isLong)); this.entryValueMovingAverageWithoutTaxesAndFees = new LazyValue<>( - () -> getMovingAverageCost(converter, TaxesAndFees.NOT_INCLUDED)); + () -> getMovingAverageCost(converter, TaxesAndFees.NOT_INCLUDED, isLong)); } private void calculateIRR(CurrencyConverter converter) @@ -332,7 +332,10 @@ public Money getProfitLoss() public Money getProfitLossMovingAverage() { - return exitValue.subtract(entryValueMovingAverage.get()); + if (isLong()) + return exitValue.subtract(entryValueMovingAverage.get()); + else + return entryValueMovingAverage.get().subtract(exitValue); } public Money getProfitLossWithoutTaxesAndFees() @@ -345,7 +348,10 @@ public Money getProfitLossWithoutTaxesAndFees() public Money getProfitLossMovingAverageWithoutTaxesAndFees() { - return exitValueWithoutTaxesAndFees.subtract(entryValueMovingAverageWithoutTaxesAndFees.get()); + if (isLong()) + return exitValueWithoutTaxesAndFees.subtract(entryValueMovingAverageWithoutTaxesAndFees.get()); + else + return entryValueMovingAverageWithoutTaxesAndFees.get().subtract(exitValueWithoutTaxesAndFees); } public long getHoldingPeriod() @@ -368,7 +374,11 @@ public double getReturn() public double getReturnMovingAverage() { - return (exitValue.getAmount() / (double) entryValueMovingAverage.get().getAmount()) - 1; + if (isLong()) + return (exitValue.getAmount() / (double) entryValueMovingAverage.get().getAmount()) - 1; + else + return 1 - (exitValue.getAmount() / (double) entryValueMovingAverage.get().getAmount()); + } /** @@ -414,7 +424,7 @@ public String toString() shares, start, entryValue, end, exitValue); } - private Money getMovingAverageCost(CurrencyConverter converter, TaxesAndFees taxesAndFees) + private Money getMovingAverageCost(CurrencyConverter converter, TaxesAndFees taxesAndFees, boolean isLong) { var holdShares = 0d; var amount = 0L; @@ -423,7 +433,7 @@ private Money getMovingAverageCost(CurrencyConverter converter, TaxesAndFees tax for (TransactionPair t : transactionsMovingAverage) { PortfolioTransaction tx = t.getTransaction(); - if (tx.getType().isPurchase()) + if (tx.getType().isPurchase() == isLong) { holdShares += tx.getShares(); amount += taxesAndFees == TaxesAndFees.INCLUDED