diff --git a/IMPROVEMENT_PLAN.md b/IMPROVEMENT_PLAN.md index 84ae1ed..990cc37 100644 --- a/IMPROVEMENT_PLAN.md +++ b/IMPROVEMENT_PLAN.md @@ -100,5 +100,6 @@ Analysis and phased plan produced 2026-07-02. See conversation history for full - **PR C** — Phase 2 mechanical cleanup: items 11, 13, 15, 16, 17. - **PR D** — Phase 2 item 9: per-request unit of work / cache. Split off from PR C because it is invasive enough to warrant its own review. - **PR E** — Phase 3 item 22: ChartService cleanup. Promoted ahead of async because it fixes an active correctness bug — `GetAccountExpensesPerMonth(accountId, year)` was mutating `ISettingsService` on the singleton to hop years, with the year parameter always hardcoded to `DateTime.Now.Year`, so the Spendings-over-time chart discarded the user's header year selection. -- **PR F** — Phase 3 items 19 + 21: async I/O across the repository stack, and precomputed dictionaries for view assembly. -- **PR G** — Phase 4 (tests) — locks in behavior for PR F. +- **PR F** — Phase 3 item 21: precomputed dictionaries in `ViewService` to eliminate O(N·M) `Single(...)` scans and per-account transaction filtering. +- **Item 19 (async I/O)** — deferred. After PR D, each XML file is loaded at most once per SignalR circuit, and this app is single-user local Blazor Server. Converting every repository/service method to async would touch ~40 files for negligible user-visible benefit and real regression risk. Revisit if the app is ever hosted for multiple concurrent users. +- **PR G** — Phase 4 (tests) — locks in behavior before further invasive changes. diff --git a/Schaad.Accounting.Services/ViewService.cs b/Schaad.Accounting.Services/ViewService.cs index 836a0d7..e4fb22c 100644 --- a/Schaad.Accounting.Services/ViewService.cs +++ b/Schaad.Accounting.Services/ViewService.cs @@ -57,22 +57,43 @@ namespace Schaad.Accounting.Services var subClassNameByNumber = subclassRepository.GetSubClassList().ToDictionary(s => s.Number, s => s.Name); var settings = settingsService.GetSettings(); - var accountList = accounts.Select( - a => - new AccountDataset( - account: a, - balance: GetBalanceInAccountCurrency(a, transactionList), - balanceCHF: GetCurrentBalanceInCHF(a, transactionList), - startBalanceCHF: fxService.ConvertCurrency(a.StartBalance, a.Currency, "CHF", settings.FixerIoApiKey), - className: subclassRepository.GetClass(a.Class), - subClassName: subClassNameByNumber[a.SubClass] - ) - ) + // Group transactions by account so per-account balance is O(k) instead of O(M). + var creditsByAccount = transactionList.GroupBy(t => t.TargetAccountId).ToDictionary(g => g.Key, g => g.ToList()); + var debitsByAccount = transactionList.GroupBy(t => t.OriginAccountId).ToDictionary(g => g.Key, g => g.ToList()); + + var accountList = accounts.Select(a => + { + var balance = ComputeBalance(a, creditsByAccount, debitsByAccount); + return new AccountDataset( + account: a, + balance: balance, + balanceCHF: fxService.ConvertCurrency(balance, a.Currency, "CHF", settings.FixerIoApiKey), + startBalanceCHF: fxService.ConvertCurrency(a.StartBalance, a.Currency, "CHF", settings.FixerIoApiKey), + className: subclassRepository.GetClass(a.Class), + subClassName: subClassNameByNumber[a.SubClass]); + }) .ToList(); return accountList.OrderBy(a => a.Number).ToList(); } + private static decimal ComputeBalance( + Account account, + Dictionary> creditsByAccount, + Dictionary> debitsByAccount) + { + var balance = account.StartBalance; + if (creditsByAccount.TryGetValue(account.Id, out var credits)) + { + balance += credits.Sum(t => t.GetValue(account.IsFxAccount)); + } + if (debitsByAccount.TryGetValue(account.Id, out var debits)) + { + balance -= debits.Sum(t => t.GetValue(account.IsFxAccount)); + } + return balance; + } + public BalanceDataset GetBalanceView() { var accountList = GetAccountViewList(); @@ -107,40 +128,20 @@ namespace Schaad.Accounting.Services return balanceView; } - private decimal GetBalanceInAccountCurrency(Account a, List transactionList) - { - var balance = a.StartBalance - + transactionList.Where(t => t.TargetAccountId == a.Id).Sum(t => t.GetValue(a.IsFxAccount)) - - transactionList.Where(t => t.OriginAccountId == a.Id).Sum(t => t.GetValue(a.IsFxAccount)); - return balance; - } - - private decimal GetCurrentBalanceInCHF(Account a, List transactionList) - { - var settings = settingsService.GetSettings(); - var balanceInAccountCurrency = GetBalanceInAccountCurrency(a, transactionList); - var balanceInChf = fxService.ConvertCurrency(balanceInAccountCurrency, a.Currency, "CHF", settings.FixerIoApiKey); - return balanceInChf; - } - /// /// Get transaction list with the origin and target account for each transaction /// public List GetTransactionViewList() { - var accountList = accountRepository.GetAccountList(); + var accountsById = accountRepository.GetAccountList().ToDictionary(a => a.Id); var transactionList = transactionRepository.GetTransactionList(); - var transactionViewList = transactionList.Select( - t => - new TransactionDataset( - t, - accountList.Single(a => a.Id == t.OriginAccountId), - accountList.Single(a => a.Id == t.TargetAccountId)) - ) + return transactionList.Select(t => + new TransactionDataset( + t, + accountsById[t.OriginAccountId], + accountsById[t.TargetAccountId])) .ToList(); - - return transactionViewList; } /// @@ -148,53 +149,41 @@ namespace Schaad.Accounting.Services /// public List GetTransactionViewList(string accountId) { - var accountList = accountRepository.GetAccountList(); - var transactionList = transactionRepository.GetTransactionList().Where(t => t.OriginAccountId == accountId || t.TargetAccountId == accountId); + var accountsById = accountRepository.GetAccountList().ToDictionary(a => a.Id); + var transactionList = transactionRepository.GetTransactionList() + .Where(t => t.OriginAccountId == accountId || t.TargetAccountId == accountId); - var transactionViewList = transactionList.Select( - t => - new TransactionDataset( - WithDisplaySign(t), - accountList.Single(a => a.Id == t.OriginAccountId), - accountList.Single(a => a.Id == t.TargetAccountId) - ) - ) + return transactionList.Select(t => + new TransactionDataset( + WithDisplaySign(t), + accountsById[t.OriginAccountId], + accountsById[t.TargetAccountId])) .ToList(); - return transactionViewList; - Transaction WithDisplaySign(Transaction t) { - var account = accountList.Single(a => a.Id == t.OriginAccountId); - - if (account.Class == ClassIds.Activa && accountId == t.OriginAccountId) + var originAccount = accountsById[t.OriginAccountId]; + if (originAccount.Class == ClassIds.Activa && accountId == t.OriginAccountId) { var copy = new Transaction(); t.Copy(copy); copy.Value *= -1; return copy; } - return t; } } - /// /// Get booking rules with their account /// public List GetBookingRuleViewList() { - var accountList = accountRepository.GetAccountList(); - var bookinRuleList = bookingRuleRepository.GetBookingRuleList(); + var accountsById = accountRepository.GetAccountList().ToDictionary(a => a.Id); + var bookingRules = bookingRuleRepository.GetBookingRuleList(); - return bookinRuleList.Select( - t => - new BookingRuleDataset( - t, - accountList.Single(a => a.Id == t.AccountId).Name - ) - ) + return bookingRules.Select(t => + new BookingRuleDataset(t, accountsById[t.AccountId].Name)) .ToList(); }