From c069c0ffa4f0df1a2c1cfe07a1248356106a353b Mon Sep 17 00:00:00 2001 From: Daniel Devaud Date: Sat, 15 Jul 2023 12:11:23 +0200 Subject: [PATCH] Refactored performance calculations - Avoiding multiple iterations over same array - Some Refactorings for readability --- .../src/app/portfolio/portfolio-calculator.ts | 56 ++--- .../src/app/portfolio/portfolio.controller.ts | 100 ++++---- .../src/app/portfolio/portfolio.service.ts | 227 ++++++++++++------ 3 files changed, 225 insertions(+), 158 deletions(-) diff --git a/apps/api/src/app/portfolio/portfolio-calculator.ts b/apps/api/src/app/portfolio/portfolio-calculator.ts index 07f7339eb..c0c042925 100644 --- a/apps/api/src/app/portfolio/portfolio-calculator.ts +++ b/apps/api/src/app/portfolio/portfolio-calculator.ts @@ -3,6 +3,7 @@ import { IDataGatheringItem } from '@ghostfolio/api/services/interfaces/interfac import { DATE_FORMAT, parseDate, resetHours } from '@ghostfolio/common/helper'; import { DataProviderInfo, + HistoricalDataItem, ResponseError, TimelinePosition } from '@ghostfolio/common/interfaces'; @@ -277,46 +278,29 @@ export class PortfolioCalculator { }; } - for (const currentDate of dates) { - const dateString = format(currentDate, DATE_FORMAT); + return dates.map((date) => { + const dateString = format(date, DATE_FORMAT); + let totalCurrentValue = new Big(0); + let totalInvestmentValue = new Big(0); + let maxTotalInvestmentValue = new Big(0); + let totalNetPerformanceValue = new Big(0); for (const symbol of Object.keys(valuesBySymbol)) { const symbolValues = valuesBySymbol[symbol]; - const currentValue = - symbolValues.currentValues?.[dateString] ?? new Big(0); - const investmentValue = - symbolValues.investmentValues?.[dateString] ?? new Big(0); - const maxInvestmentValue = - symbolValues.maxInvestmentValues?.[dateString] ?? new Big(0); - const netPerformanceValue = - symbolValues.netPerformanceValues?.[dateString] ?? new Big(0); - - valuesByDate[dateString] = { - totalCurrentValue: ( - valuesByDate[dateString]?.totalCurrentValue ?? new Big(0) - ).add(currentValue), - totalInvestmentValue: ( - valuesByDate[dateString]?.totalInvestmentValue ?? new Big(0) - ).add(investmentValue), - maxTotalInvestmentValue: ( - valuesByDate[dateString]?.maxTotalInvestmentValue ?? new Big(0) - ).add(maxInvestmentValue), - totalNetPerformanceValue: ( - valuesByDate[dateString]?.totalNetPerformanceValue ?? new Big(0) - ).add(netPerformanceValue) - }; + totalCurrentValue = totalCurrentValue.plus( + symbolValues.currentValues?.[dateString] ?? new Big(0) + ); + totalInvestmentValue = totalInvestmentValue.plus( + symbolValues.investmentValues?.[dateString] ?? new Big(0) + ); + maxTotalInvestmentValue = maxTotalInvestmentValue.plus( + symbolValues.maxInvestmentValues?.[dateString] ?? new Big(0) + ); + totalNetPerformanceValue = totalNetPerformanceValue.plus( + symbolValues.netPerformanceValues?.[dateString] ?? new Big(0) + ); } - } - - return Object.entries(valuesByDate).map(([date, values]) => { - const { - maxTotalInvestmentValue, - totalCurrentValue, - totalInvestmentValue, - totalNetPerformanceValue - } = values; - const netPerformanceInPercentage = maxTotalInvestmentValue.eq(0) ? 0 : totalNetPerformanceValue @@ -325,7 +309,7 @@ export class PortfolioCalculator { .toNumber(); return { - date, + date: dateString, netPerformanceInPercentage, netPerformance: totalNetPerformanceValue.toNumber(), totalInvestment: totalInvestmentValue.toNumber(), diff --git a/apps/api/src/app/portfolio/portfolio.controller.ts b/apps/api/src/app/portfolio/portfolio.controller.ts index 06f841e12..d1f76a97c 100644 --- a/apps/api/src/app/portfolio/portfolio.controller.ts +++ b/apps/api/src/app/portfolio/portfolio.controller.ts @@ -111,21 +111,38 @@ export class PortfolioController { impersonationId || this.userService.isRestrictedView(this.request.user) ) { - const totalInvestment = Object.values(holdings) - .map((portfolioPosition) => { - return portfolioPosition.investment; - }) - .reduce((a, b) => a + b, 0); - - const totalValue = Object.values(holdings) - .map((portfolioPosition) => { - return this.exchangeRateDataService.toCurrency( - portfolioPosition.quantity * portfolioPosition.marketPrice, - portfolioPosition.currency, - this.request.user.Settings.settings.baseCurrency - ); - }) - .reduce((a, b) => a + b, 0); + let investmentTuple: [number, number] = [0, 0]; + for (let holding of Object.entries(holdings)) { + var portfolioPosition = holding[1]; + investmentTuple[0] += portfolioPosition.investment; + investmentTuple[1] += this.exchangeRateDataService.toCurrency( + portfolioPosition.quantity * portfolioPosition.marketPrice, + portfolioPosition.currency, + this.request.user.Settings.settings.baseCurrency + ); + } + const totalInvestment = investmentTuple[0]; + + const totalValue = investmentTuple[1]; + + if (hasDetails === false) { + portfolioSummary = nullifyValuesInObject(summary, [ + 'cash', + 'committedFunds', + 'currentGrossPerformance', + 'currentNetPerformance', + 'currentValue', + 'dividend', + 'emergencyFund', + 'excludedAccountsAndActivities', + 'fees', + 'items', + 'liabilities', + 'netWorth', + 'totalBuy', + 'totalSell' + ]); + } for (const [symbol, portfolioPosition] of Object.entries(holdings)) { portfolioPosition.grossPerformance = null; @@ -135,6 +152,24 @@ export class PortfolioController { portfolioPosition.quantity = null; portfolioPosition.valueInPercentage = portfolioPosition.value / totalValue; + (portfolioPosition.assetClass = hasDetails + ? portfolioPosition.assetClass + : undefined), + (portfolioPosition.assetSubClass = hasDetails + ? portfolioPosition.assetSubClass + : undefined), + (portfolioPosition.countries = hasDetails + ? portfolioPosition.countries + : []), + (portfolioPosition.currency = hasDetails + ? portfolioPosition.currency + : undefined), + (portfolioPosition.markets = hasDetails + ? portfolioPosition.markets + : undefined), + (portfolioPosition.sectors = hasDetails + ? portfolioPosition.sectors + : []); } for (const [name, { valueInBaseCurrency }] of Object.entries(accounts)) { @@ -146,41 +181,6 @@ export class PortfolioController { } } - if ( - hasDetails === false || - impersonationId || - this.userService.isRestrictedView(this.request.user) - ) { - portfolioSummary = nullifyValuesInObject(summary, [ - 'cash', - 'committedFunds', - 'currentGrossPerformance', - 'currentNetPerformance', - 'currentValue', - 'dividend', - 'emergencyFund', - 'excludedAccountsAndActivities', - 'fees', - 'items', - 'liabilities', - 'netWorth', - 'totalBuy', - 'totalSell' - ]); - } - - for (const [symbol, portfolioPosition] of Object.entries(holdings)) { - holdings[symbol] = { - ...portfolioPosition, - assetClass: hasDetails ? portfolioPosition.assetClass : undefined, - assetSubClass: hasDetails ? portfolioPosition.assetSubClass : undefined, - countries: hasDetails ? portfolioPosition.countries : [], - currency: hasDetails ? portfolioPosition.currency : undefined, - markets: hasDetails ? portfolioPosition.markets : undefined, - sectors: hasDetails ? portfolioPosition.sectors : [] - }; - } - return { accounts, filteredValueInBaseCurrency, diff --git a/apps/api/src/app/portfolio/portfolio.service.ts b/apps/api/src/app/portfolio/portfolio.service.ts index 2d0e0ac23..feb70c861 100644 --- a/apps/api/src/app/portfolio/portfolio.service.ts +++ b/apps/api/src/app/portfolio/portfolio.service.ts @@ -522,11 +522,9 @@ export class PortfolioService { } const portfolioItemsNow: { [symbol: string]: TimelinePosition } = {}; - for (const position of currentPositions.positions) { - portfolioItemsNow[position.symbol] = position; - } for (const item of currentPositions.positions) { + portfolioItemsNow[item.symbol] = item; if (item.quantity.lte(0)) { // Ignore positions without any quantity continue; @@ -542,21 +540,7 @@ export class PortfolioService { otherMarkets: 0 }; - for (const country of symbolProfile.countries) { - if (developedMarkets.includes(country.code)) { - markets.developedMarkets = new Big(markets.developedMarkets) - .plus(country.weight) - .toNumber(); - } else if (emergingMarkets.includes(country.code)) { - markets.emergingMarkets = new Big(markets.emergingMarkets) - .plus(country.weight) - .toNumber(); - } else { - markets.otherMarkets = new Big(markets.otherMarkets) - .plus(country.weight) - .toNumber(); - } - } + this.calculateMarketsAllocation(symbolProfile, markets); holdings[item.symbol] = { markets, @@ -587,6 +571,68 @@ export class PortfolioService { }; } + await this.handleCashPosition( + filters, + isFilteredByAccount, + cashDetails, + userCurrency, + filteredValueInBaseCurrency, + holdings + ); + + const { accounts, platforms } = await this.getValueOfAccountsAndPlatforms({ + filters, + orders, + portfolioItemsNow, + userCurrency, + userId, + withExcludedAccounts + }); + + filteredValueInBaseCurrency = await this.handleEmergencyFunds( + filters, + cashDetails, + userCurrency, + filteredValueInBaseCurrency, + emergencyFund, + orders, + accounts, + holdings + ); + + const summary = await this.getSummary({ + impersonationId, + userCurrency, + userId, + balanceInBaseCurrency: cashDetails.balanceInBaseCurrency, + emergencyFundPositionsValueInBaseCurrency: + this.getEmergencyFundPositionsValueInBaseCurrency({ + activities: orders + }) + }); + + return { + accounts, + holdings, + platforms, + summary, + filteredValueInBaseCurrency: filteredValueInBaseCurrency.toNumber(), + filteredValueInPercentage: summary.netWorth + ? filteredValueInBaseCurrency.div(summary.netWorth).toNumber() + : 0, + hasErrors: currentPositions.hasErrors, + totalValueInBaseCurrency: summary.netWorth + }; + } + + private async handleCashPosition( + filters: Filter[], + isFilteredByAccount: boolean, + cashDetails: CashDetails, + userCurrency: string, + filteredValueInBaseCurrency: Big, + holdings: { [symbol: string]: PortfolioPosition } + ) { const isFilteredByCash = filters?.some((filter) => { return filter.type === 'ASSET_CLASS' && filter.id === 'CASH'; }); @@ -602,16 +648,26 @@ export class PortfolioService { holdings[symbol] = cashPositions[symbol]; } } + } - const { accounts, platforms } = await this.getValueOfAccountsAndPlatforms({ - filters, - orders, - portfolioItemsNow, - userCurrency, - userId, - withExcludedAccounts - }); - + private async handleEmergencyFunds( + filters: Filter[], + cashDetails: CashDetails, + userCurrency: string, + filteredValueInBaseCurrency: Big, + emergencyFund: Big, + orders: Activity[], + accounts: { + [id: string]: { + balance: number; + currency: string; + name: string; + valueInBaseCurrency: number; + valueInPercentage?: number; + }; + }, + holdings: { [symbol: string]: PortfolioPosition } + ) { if ( filters?.length === 1 && filters[0].id === EMERGENCY_FUND_TAG_ID && @@ -646,30 +702,32 @@ export class PortfolioService { value: emergencyFundInCash }; } + return filteredValueInBaseCurrency; + } - const summary = await this.getSummary({ - impersonationId, - userCurrency, - userId, - balanceInBaseCurrency: cashDetails.balanceInBaseCurrency, - emergencyFundPositionsValueInBaseCurrency: - this.getEmergencyFundPositionsValueInBaseCurrency({ - activities: orders - }) - }); - - return { - accounts, - holdings, - platforms, - summary, - filteredValueInBaseCurrency: filteredValueInBaseCurrency.toNumber(), - filteredValueInPercentage: summary.netWorth - ? filteredValueInBaseCurrency.div(summary.netWorth).toNumber() - : 0, - hasErrors: currentPositions.hasErrors, - totalValueInBaseCurrency: summary.netWorth - }; + private calculateMarketsAllocation( + symbolProfile: EnhancedSymbolProfile, + markets: { + developedMarkets: number; + emergingMarkets: number; + otherMarkets: number; + } + ) { + for (const country of symbolProfile.countries) { + if (developedMarkets.includes(country.code)) { + markets.developedMarkets = new Big(markets.developedMarkets) + .plus(country.weight) + .toNumber(); + } else if (emergingMarkets.includes(country.code)) { + markets.emergingMarkets = new Big(markets.emergingMarkets) + .plus(country.weight) + .toNumber(); + } else { + markets.otherMarkets = new Big(markets.otherMarkets) + .plus(country.weight) + .toNumber(); + } + } } public async getPosition( @@ -1607,38 +1665,63 @@ export class PortfolioService { userId }); - const activities = await this.orderService.getOrders({ + const ordersRaw = await this.orderService.getOrders({ userCurrency, - userId - }); - - const excludedActivities = ( - await this.orderService.getOrders({ - userCurrency, - userId, - withExcludedAccounts: true - }) - ).filter(({ Account: account }) => { - return account?.isExcluded ?? false; + userId, + withExcludedAccounts: true }); + const activities: Activity[] = []; + const excludedActivities: Activity[] = []; + let dividend = 0; + let fees = 0; + let items = 0; + + let liabilities = 0; + + let totalBuy = 0; + let totalSell = 0; + for (let order of ordersRaw) { + if (order.Account?.isExcluded ?? false) { + excludedActivities.push(order); + } else { + activities.push(order); + fees += this.exchangeRateDataService.toCurrency( + order.fee, + order.SymbolProfile.currency, + userCurrency + ); + let amount = this.exchangeRateDataService.toCurrency( + new Big(order.quantity).mul(order.unitPrice).toNumber(), + order.SymbolProfile.currency, + userCurrency + ); + switch (order.type) { + case 'DIVIDEND': + dividend += amount; + break; + case 'ITEM': + items += amount; + break; + case 'SELL': + totalSell += amount; + break; + case 'BUY': + totalBuy += amount; + break; + case 'LIABILITY': + liabilities += amount; + } + } + } - const dividend = this.getDividend({ - activities, - userCurrency - }).toNumber(); const emergencyFund = new Big( Math.max( emergencyFundPositionsValueInBaseCurrency, (user.Settings?.settings as UserSettings)?.emergencyFund ?? 0 ) ); - const fees = this.getFees({ activities, userCurrency }).toNumber(); - const firstOrderDate = activities[0]?.date; - const items = this.getItems(activities).toNumber(); - const liabilities = this.getLiabilities(activities).toNumber(); - const totalBuy = this.getTotalByType(activities, userCurrency, 'BUY'); - const totalSell = this.getTotalByType(activities, userCurrency, 'SELL'); + const firstOrderDate = activities[0]?.date; const cash = new Big(balanceInBaseCurrency) .minus(emergencyFund)