PR L: drop Copy(target) methods in favour of index-based replace

Replace the hand-rolled model.Copy(target) pattern in every SaveXxx
with FindIndex -> in-place replace (or Add on new). Move the two
hidden defaults out of the deleted Copy bodies: Currency = "CHF"
default now lives in AccountRepository.SaveAccount; BookingDate =
ValueDate default now lives in TransactionRepository.SaveTransaction.

Delete Copy from Account, BookingRule, BookingText, SubClass, and
SplitPredefinition. Keep Transaction.Clone() (renamed from Copy, and
now includes RelatedParty) for the defensive copy in GetTransaction
and ViewService.WithDisplaySign.

Fixes a pre-existing bug where Transaction.Copy silently dropped
RelatedParty on every update save.

42 tests total, all passing.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

IMPROVEMENT_PLAN.md

@@ -107,3 +107,4 @@ Analysis and phased plan produced 2026-07-02. See conversation history for full
 - **PR I** — Phase 2 item 12: move the service interfaces (`IViewService`, `IFileService`, `IChartService`) from `Schaad.Accounting.Common` into `Schaad.Accounting.Services/Interfaces/`. Namespaces are unchanged (`Schaad.Accounting.Interfaces`), so no consumer needs a `using` update. Drops the `Schaad.Finance.Api` `<Reference>` from `Common.csproj` — Common is now vendor-free and matches its documented role as the "shared models, DTOs, interfaces" layer.
 - **PR J** — Phase 2 item 18 (second slice): thread `ILogger<T>` through `BaseRepository` and every concrete repository so `BaseRepository.Save`'s catch block logs the failing file path and exception before rethrowing (previously the exception's origin was silently swallowed and only the stack trace at the callsite survived). Pulls `Microsoft.Extensions.Logging.Abstractions` into the Db project. `TransactionRepositoryTestShould` and `AccountRepositoryTestShould` use `NullLogger<T>.Instance`. Remaining item 18 work (MatchOpenBankTransactions match-count summary in `ViewService`) still open.
 - **PR K** — Phase 5 item 27: introduce `IFxConverter.ConvertToChf(amount, fromCurrency)` and its `FxConverter` implementation. The vendor `IFxService` and the FixerIo API key are now hidden inside `FxConverter`; callers stop threading the API key through every method call. `ViewService` drops both `IFxService` and `ISettingsService` from its constructor and takes `IFxConverter` instead. Simplifies `GetAccountViewList` (no more per-call `settingsService.GetSettings()` reads) and `GetBalanceSheetView`. Two-test `FxConverterTestShould` locks in the target-currency and API-key routing.
+- **PR L** — Phase 2 item 10: drop the hand-rolled `Copy(target)` methods from `Account`, `BookingRule`, `BookingText`, `SubClass`, and `SplitPredefinition`; keep only a `Transaction.Clone()` for defensive-copy needs in `GetTransaction` / `ViewService.WithDisplaySign`. Every `SaveXxx` now uses `FindIndex` → in-place replace (or `Add` for new entries), moving the two "hidden" defaults (`Account.Currency = "CHF"` when unset, `Transaction.BookingDate = ValueDate` when unset) into the corresponding `SaveXxx` method where they belong. Fixes a pre-existing bug: `Transaction.Copy` never copied `RelatedParty`, so update-saves silently dropped it. Added `PreserveRelatedPartyWhenRoundTrippingTransaction` and `DefaultBookingDateToValueDateWhenBookingDateIsUnset` as regression tests. 42 tests total.

Schaad.Accounting.Common/Models/Account.cs

@@ -59,19 +59,5 @@ namespace Schaad.Accounting.Models
         {
             get { return string.IsNullOrEmpty(Currency) == false && Currency != "CHF"; }
         }
-
-        /// <summary>
-        /// Makes a copy
-        /// </summary>
-        public void Copy(Account target)
-        {
-            target.LastBankBalance = LastBankBalance;
-            target.StartBalance = StartBalance;
-            target.BankAccountNumber = BankAccountNumber;
-            target.Currency = string.IsNullOrEmpty(Currency) ? "CHF" : Currency;
-            target.Id = Id;
-            target.Name = Name;
-            target.Number = Number;
-        }
     }
 }

Schaad.Accounting.Common/Models/BookingRule.cs

@@ -20,18 +20,5 @@ namespace Schaad.Accounting.Models
         [Display(Name = "Konto")]
         [Required]
         public string AccountId { get; set; }
-
-
-        /// <summary>
-        /// Makes a copy
-        /// </summary>
-        public void Copy(BookingRule target)
-        {
-            target.LookupText = LookupText;
-            target.LookupValue = LookupValue;
-            target.BookingText = BookingText;
-            target.Id = Id;
-            target.AccountId = AccountId;
-        }
     }
 }

Schaad.Accounting.Common/Models/BookingText.cs

@@ -10,14 +10,5 @@ namespace Schaad.Accounting.Models
         [Required]
         [MinLength(3)]
         public string Text { get; set; }
-
-        /// <summary>
-        /// Makes a copy
-        /// </summary>
-        public void Copy(BookingText target)
-        {
-            target.Id = Id;
-            target.Text = Text;
-        }
     }
 }

Schaad.Accounting.Common/Models/SplitPredefinition.cs

@@ -9,17 +9,5 @@
         public decimal BookingValue { get; set; }
 
         public string AccountId { get; set; }
-
-
-        /// <summary>
-        /// Makes a copy
-        /// </summary>
-        public void Copy(SplitPredefinition target)
-        {
-            target.BookingText = BookingText;
-            target.BookingValue = BookingValue;
-            target.Id = Id;
-            target.AccountId = AccountId;
-        }
     }
 }

Schaad.Accounting.Common/Models/SubClass.cs

@@ -14,15 +14,5 @@ namespace Schaad.Accounting.Models
         [Required]
         [MinLength(3)]
         public string Name { get; set; }
-
-        /// <summary>
-        /// Makes a copy
-        /// </summary>
-        public void Copy(SubClass target)
-        {
-            target.Id = Id;
-            target.Name = Name;
-            target.Number = Number;
-        }
     }
 }

Schaad.Accounting.Common/Models/Transaction.cs

@@ -74,20 +74,22 @@ namespace Schaad.Accounting.Models
         }
 
         /// <summary>
-        /// Makes a copy
+        /// Returns a shallow independent copy — used to hand out defensive copies
+        /// (e.g. from repository reads) without exposing the caller to later mutation.
         /// </summary>
-        public void Copy(Transaction target)
+        public Transaction Clone() => new()
         {
-            target.BankTransactionId = BankTransactionId;
+            Id = Id,
-            target.BankTransactionText = BankTransactionText;
+            BankTransactionId = BankTransactionId,
-            target.Id = Id;
+            BankTransactionText = BankTransactionText,
-            target.OriginAccountId = OriginAccountId;
+            RelatedParty = RelatedParty,
-            target.TargetAccountId = TargetAccountId;
+            OriginAccountId = OriginAccountId,
-            target.Text = Text;
+            TargetAccountId = TargetAccountId,
-            target.Value = Value;
+            Text = Text,
-            target.ValueDate = ValueDate;
+            Value = Value,
-            target.BookingDate = BookingDate > DateTime.MinValue ? BookingDate : ValueDate;
+            ValueDate = ValueDate,
-            target.FxRate = FxRate;
+            BookingDate = BookingDate,
-        }
+            FxRate = FxRate
+        };
     }
 }

Schaad.Accounting.Db/Repositories/AccountRepository.cs

@@ -28,16 +28,22 @@ namespace Schaad.Accounting.Repositories
         /// </summary>
         public void SaveAccount(Account account)
         {
-            var accounts = GetAccountList();
+            if (string.IsNullOrEmpty(account.Currency))
-            var existingAccount = accounts.FirstOrDefault(a => a.Id == account.Id);
-
-            if (existingAccount == null)
             {
-                existingAccount = new Account();
+                account.Currency = "CHF";
-                accounts.Add(existingAccount);
+            }
-                account.Id = Guid.NewGuid().ToString();
+
+            var accounts = GetAccountList();
+            var idx = accounts.FindIndex(a => a.Id == account.Id);
+            if (idx >= 0)
+            {
+                accounts[idx] = account;
+            }
+            else
+            {
+                account.Id = Guid.NewGuid().ToString();
+                accounts.Add(account);
             }
-            account.Copy(existingAccount);
             Save(accounts, ACCOUNTS);
         }
 

Schaad.Accounting.Db/Repositories/BookingRuleRepository.cs

@@ -28,15 +28,16 @@ namespace Schaad.Accounting.Repositories
         public void SaveBookingRule(BookingRule bookingRule)
         {
             var bookingRules = GetBookingRuleList();
-            var existingRule = bookingRules.FirstOrDefault(a => a.Id == bookingRule.Id);
+            var idx = bookingRules.FindIndex(r => r.Id == bookingRule.Id);
-
+            if (idx >= 0)
-            if (existingRule == null)
             {
-                existingRule = new BookingRule();
+                bookingRules[idx] = bookingRule;
-                bookingRules.Add(existingRule);
+            }
-                bookingRule.Id = Guid.NewGuid().ToString();
+            else
+            {
+                bookingRule.Id = Guid.NewGuid().ToString();
+                bookingRules.Add(bookingRule);
             }
-            bookingRule.Copy(existingRule);
             Save(bookingRules, BOOKING_RULES);
         }
 

Schaad.Accounting.Db/Repositories/BookingTextRepository.cs

@@ -28,15 +28,16 @@ namespace Schaad.Accounting.Repositories
         public void SaveBookingText(BookingText bookingText)
         {
             var bookingTexts = GetBookingTextList();
-            var existingText = bookingTexts.FirstOrDefault(a => a.Id == bookingText.Id);
+            var idx = bookingTexts.FindIndex(t => t.Id == bookingText.Id);
-
+            if (idx >= 0)
-            if (existingText == null)
             {
-                existingText = new BookingText();
+                bookingTexts[idx] = bookingText;
-                bookingTexts.Add(existingText);
+            }
-                bookingText.Id = Guid.NewGuid().ToString();
+            else
+            {
+                bookingText.Id = Guid.NewGuid().ToString();
+                bookingTexts.Add(bookingText);
             }
-            bookingText.Copy(existingText);
             Save(bookingTexts, BOOKING_TEXTS);
         }
 

Schaad.Accounting.Db/Repositories/SplitPredefinitionRepository.cs

@@ -28,15 +28,16 @@ namespace Schaad.Accounting.Repositories
         public void SaveSplitPredefinition(SplitPredefinition splitPredefinition)
         {
             var definitions = GetSplitPredefinitionList();
-            var existingDefinition = definitions.FirstOrDefault(a => a.Id == splitPredefinition.Id);
+            var idx = definitions.FindIndex(d => d.Id == splitPredefinition.Id);
-
+            if (idx >= 0)
-            if (existingDefinition == null)
             {
-                existingDefinition = new SplitPredefinition();
+                definitions[idx] = splitPredefinition;
-                definitions.Add(existingDefinition);
+            }
-                splitPredefinition.Id = Guid.NewGuid().ToString();
+            else
+            {
+                splitPredefinition.Id = Guid.NewGuid().ToString();
+                definitions.Add(splitPredefinition);
             }
-            splitPredefinition.Copy(existingDefinition);
             Save(definitions, SPLIT_PREDEFINITION);
         }
     }

Schaad.Accounting.Db/Repositories/SubclassRepository.cs

@@ -29,20 +29,21 @@ namespace Schaad.Accounting.Repositories
         public List<SubClass> GetSubClassList() => LoadList<SubClass>(SUBCLASSES);
 
         /// <summary>
-        /// Save a booking text (insert/update)
+        /// Save a subclass (insert/update)
         /// </summary>
         public void SaveSubClass(SubClass subClass)
         {
             var subclasses = GetSubClassList();
-            var existingSubClass = subclasses.FirstOrDefault(a => a.Id == subClass.Id);
+            var idx = subclasses.FindIndex(s => s.Id == subClass.Id);
-
+            if (idx >= 0)
-            if (existingSubClass == null)
             {
-                existingSubClass = new SubClass();
+                subclasses[idx] = subClass;
-                subclasses.Add(existingSubClass);
+            }
-                subClass.Id = Guid.NewGuid().ToString();
+            else
+            {
+                subClass.Id = Guid.NewGuid().ToString();
+                subclasses.Add(subClass);
             }
-            subClass.Copy(existingSubClass);
             Save(subclasses, SUBCLASSES);
         }
 

Schaad.Accounting.Db/Repositories/TransactionRepository.cs

@@ -45,17 +45,22 @@ namespace Schaad.Accounting.Repositories
                 transaction.FxRate = null;
             }
 
+            if (transaction.BookingDate <= DateTime.MinValue)
+            {
+                transaction.BookingDate = transaction.ValueDate;
+            }
 
             var transactionList = GetTransactionList();
-            var existingTransaction = transactionList.FirstOrDefault(a => a.Id == transaction.Id);
+            var idx = transactionList.FindIndex(t => t.Id == transaction.Id);
-
+            if (idx >= 0)
-            if (existingTransaction == null)
             {
-                existingTransaction = new Transaction();
+                transactionList[idx] = transaction;
-                transactionList.Add(existingTransaction);
+            }
-                transaction.Id = Guid.NewGuid().ToString();
+            else
+            {
+                transaction.Id = Guid.NewGuid().ToString();
+                transactionList.Add(transaction);
             }
-            transaction.Copy(existingTransaction);
             Save(transactionList, TRANSACTIONS);
         }
 
@@ -71,8 +76,7 @@ namespace Schaad.Accounting.Repositories
                 return null;
             }
 
-            var result = new Transaction();
+            var result = stored.Clone();
-            stored.Copy(result);
 
             // value is stored in CHF -> convert back to foreign currency for display/editing
             var isFxAccount = accountRepository.GetAccount(result.OriginAccountId).IsFxAccount

Schaad.Accounting.Services/ViewService.cs

@@ -159,8 +159,7 @@ namespace Schaad.Accounting.Services
                 var originAccount = accountsById[t.OriginAccountId];
                 if (originAccount.Class == ClassIds.Activa && accountId == t.OriginAccountId)
                 {
-                    var copy = new Transaction();
+                    var copy = t.Clone();
-                    t.Copy(copy);
                     copy.Value *= -1;
                     return copy;
                 }

Schaad.Accounting.Tests/TransactionRepositoryTestShould.cs

@@ -136,4 +136,43 @@ public class TransactionRepositoryTestShould : IDisposable
 
         sut.GetTransactionList().ShouldBeEmpty();
     }
+
+    [Fact]
+    public void PreserveRelatedPartyWhenRoundTrippingTransaction()
+    {
+        accountRepo.GetAccount(Arg.Any<string>()).Returns(new Account { Currency = "CHF" });
+
+        sut.SaveTransaction(new Transaction
+        {
+            OriginAccountId = "a",
+            TargetAccountId = "b",
+            Value = 10m,
+            Text = "Rent",
+            RelatedParty = "ACME Property AG",
+            ValueDate = new DateTime(2026, 4, 1),
+            BookingDate = new DateTime(2026, 4, 1)
+        });
+
+        // Regression: earlier Transaction.Copy dropped RelatedParty on every save,
+        // so the round-trip lost it.
+        sut.GetTransactionList().Single().RelatedParty.ShouldBe("ACME Property AG");
+    }
+
+    [Fact]
+    public void DefaultBookingDateToValueDateWhenBookingDateIsUnset()
+    {
+        accountRepo.GetAccount(Arg.Any<string>()).Returns(new Account { Currency = "CHF" });
+
+        sut.SaveTransaction(new Transaction
+        {
+            OriginAccountId = "a",
+            TargetAccountId = "b",
+            Value = 10m,
+            Text = "x",
+            ValueDate = new DateTime(2026, 4, 15)
+            // BookingDate left at default (DateTime.MinValue)
+        });
+
+        sut.GetTransactionList().Single().BookingDate.ShouldBe(new DateTime(2026, 4, 15));
+    }
 }