Skip to content

Commit e81cdf5

Browse files
AlexCatarinoclaude
AlexCatarino
and
claude
authored
Allow SetAccountCurrency after SetCash without throwing (#9457)
* Allow SetAccountCurrency after SetCash without throwing Previously, calling SetAccountCurrency after SetCash threw an InvalidOperationException. The portfolio manager now switches the base account currency in place: the previous Cash entry (and its balance) is preserved in the CashBook, and a notice is logged. When the new account currency matches the existing one, an optional startingCash overrides the previously set amount and the override is logged. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Simplify SetAccountCurrency remarks and drop unused message Tightens the XML remarks to two cases (different currency: keep previous in its own entry; matching currency: override). Also removes the now unused CannotChangeAccountCurrencyAfterSettingCash string since the portfolio no longer throws. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Simplify in-line comment in SetAccountCurrency Drops the "Undo that migration" wording (residue from a previous fix attempt) and just describes what the branch does: keep the previous balance in its own currency entry while the new account currency starts at zero. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Differentiate implicit vs explicit SetCash on account currency switch SetCash(decimal) is currency-agnostic: the amount is "this many units of the (eventual) account currency". Switching the account currency now re-labels the amount onto the new currency instead of preserving the previous one. SetCash(symbol, ...) for the current account currency keeps the old behaviour of preserving the balance in its own CashBook entry. Splits the existing tests by overload and adds a starting-cash variant for the explicit case. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Mark base cash explicit on any SetCash(symbol, ...) call Broaden _baseCashSymbolSetExplicitly so any call to the explicit-currency overload signals the user is committing to named currencies, not just calls whose symbol matches the current account currency. SetAccountCurrency then preserves the previous base-currency balance in its own CashBook entry whenever the user has touched the explicit overload at all. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 9d464c0 commit e81cdf5

3 files changed

Lines changed: 130 additions & 30 deletions

File tree

‎Common/Messages/Messages.Securities.cs‎

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -890,11 +890,23 @@ public static class SecurityPortfolioManager
890890
"Cannot change AccountCurrency after adding a Security. Please move SetAccountCurrency() before AddSecurity().";
891891

892892
/// <summary>
893-
/// Returns a string message saying the AccountCurrency cannot be changed after setting cash and that the method
894-
/// SetAccountCurrency() should be moved before SetCash()
893+
/// Returns a string message saying the AccountCurrency has been changed after setting cash, reporting the
894+
/// remaining amount held in the previous account currency
895895
/// </summary>
896-
public static string CannotChangeAccountCurrencyAfterSettingCash =
897-
"Cannot change AccountCurrency after setting cash. Please move SetAccountCurrency() before SetCash().";
896+
[MethodImpl(MethodImplOptions.AggressiveInlining)]
897+
public static string AccountCurrencyChangedAfterSettingCash(Securities.Cash previousCash)
898+
{
899+
return Invariant($"Account currency was changed after SetCash() was called. Algorithm still holds {previousCash.Amount} {previousCash.Symbol} in the previous account currency.");
900+
}
901+
902+
/// <summary>
903+
/// Returns a string message saying the account currency starting cash has been updated from a previous amount to a new one
904+
/// </summary>
905+
[MethodImpl(MethodImplOptions.AggressiveInlining)]
906+
public static string AccountCurrencyCashUpdated(string accountCurrency, decimal previousAmount, decimal newAmount)
907+
{
908+
return Invariant($"Account currency cash updated to {newAmount} {accountCurrency} from {previousAmount} {accountCurrency}.");
909+
}
898910

899911
/// <summary>
900912
/// Returns a string message saying the AccountCurrency has already been set and that the new value for this property

‎Common/Securities/SecurityPortfolioManager.cs‎

Lines changed: 36 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ public class SecurityPortfolioManager : ExtendedDictionary<Symbol, SecurityHoldi
3636
{
3737
private Cash _baseCurrencyCash;
3838
private bool _setCashWasCalled;
39+
private bool _baseCashSymbolSetExplicitly;
3940
private decimal _totalPortfolioValue;
4041
private bool _isTotalPortfolioValueValid;
4142
private object _totalPortfolioValueLock = new();
@@ -616,10 +617,15 @@ public override SecurityHolding this[Symbol symbol]
616617

617618
/// <summary>
618619
/// Sets the account currency cash symbol this algorithm is to manage, as well
619-
/// as the starting cash in this currency if given
620+
/// as the starting cash in this currency if given.
620621
/// </summary>
621-
/// <remarks>Has to be called before calling <see cref="SetCash(decimal)"/>
622-
/// or adding any <see cref="Security"/></remarks>
622+
/// <remarks>
623+
/// Should be called before adding any <see cref="Security"/>. If <see cref="SetCash(string, decimal, decimal)"/>
624+
/// was called beforehand, the previous base cash balance is kept in its own
625+
/// <see cref="CashBook"/> entry and the new account currency starts at zero. Otherwise the
626+
/// previously set amount carries over to the new account currency. If the currency matches,
627+
/// <paramref name="startingCash"/> overrides the previous amount.
628+
/// </remarks>
623629
/// <param name="accountCurrency">The account currency cash symbol to set</param>
624630
/// <param name="startingCash">The account currency starting cash to set</param>
625631
public void SetAccountCurrency(string accountCurrency, decimal? startingCash = null)
@@ -645,24 +651,39 @@ public void SetAccountCurrency(string accountCurrency, decimal? startingCash = n
645651
Messages.SecurityPortfolioManager.CannotChangeAccountCurrencyAfterAddingSecurity);
646652
}
647653

648-
if (_setCashWasCalled)
649-
{
650-
throw new InvalidOperationException("SecurityPortfolioManager.SetAccountCurrency(): " +
651-
Messages.SecurityPortfolioManager.CannotChangeAccountCurrencyAfterSettingCash);
652-
}
653-
654-
Log.Trace("SecurityPortfolioManager.SetAccountCurrency(): " +
655-
Messages.SecurityPortfolioManager.SettingAccountCurrency(accountCurrency));
654+
// Capture the previous base cash and amount if SetCash() was called earlier so we can
655+
// either report the leftover balance in the old currency or detect a same-currency override.
656+
var previousCash = _setCashWasCalled ? _baseCurrencyCash : null;
657+
var previousAmount = previousCash?.Amount;
658+
var message = Messages.SecurityPortfolioManager.SettingAccountCurrency(accountCurrency);
656659

657660
UnsettledCashBook.AccountCurrency = accountCurrency;
658661
CashBook.AccountCurrency = accountCurrency;
659662

663+
// Repoint the base cash to the new account currency entry.
660664
_baseCurrencyCash = CashBook[accountCurrency];
661665

666+
if (_baseCashSymbolSetExplicitly && previousCash != null && previousCash.Symbol != accountCurrency)
667+
{
668+
// The user committed cash to a specific currency via SetCash(symbol, ...): keep that
669+
// balance in its own entry and start the new account currency at zero. Otherwise the
670+
// CashBook setter has already migrated the implicit amount to the new currency entry.
671+
_baseCurrencyCash.SetAmount(0);
672+
CashBook.Add(previousCash.Symbol, previousAmount.Value, previousCash.ConversionRate);
673+
message += ". " + Messages.SecurityPortfolioManager.AccountCurrencyChangedAfterSettingCash(previousCash);
674+
}
675+
662676
if (startingCash != null)
663677
{
664-
SetCash((decimal)startingCash);
678+
SetCash(startingCash.Value);
679+
// When the account currency is unchanged, report the override of the prior amount.
680+
if (previousCash?.Symbol == accountCurrency && previousAmount != startingCash)
681+
{
682+
message = Messages.SecurityPortfolioManager.AccountCurrencyCashUpdated(accountCurrency, previousAmount.Value, startingCash.Value);
683+
}
665684
}
685+
686+
Log.Trace("SecurityPortfolioManager.SetAccountCurrency(): " + message);
666687
}
667688

668689
/// <summary>
@@ -672,6 +693,7 @@ public void SetAccountCurrency(string accountCurrency, decimal? startingCash = n
672693
public void SetCash(decimal cash)
673694
{
674695
_setCashWasCalled = true;
696+
_baseCashSymbolSetExplicitly = false;
675697
_baseCurrencyCash.SetAmount(cash);
676698
}
677699

@@ -684,9 +706,9 @@ public void SetCash(decimal cash)
684706
public void SetCash(string symbol, decimal cash, decimal conversionRate)
685707
{
686708
_setCashWasCalled = true;
687-
Cash item;
709+
_baseCashSymbolSetExplicitly = true;
688710
symbol = symbol.LazyToUpper();
689-
if (CashBook.TryGetValue(symbol, out item))
711+
if (CashBook.TryGetValue(symbol, out var item))
690712
{
691713
item.SetAmount(cash);
692714
item.ConversionRate = conversionRate;

‎Tests/Common/Securities/SecurityPortfolioManagerTests.cs‎

Lines changed: 78 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -2576,20 +2576,86 @@ public void CanNotChangeAccountCurrencyAfterAddingASecurity()
25762576
Assert.Throws<InvalidOperationException>(() => algorithm.Portfolio.SetAccountCurrency(Currencies.USD));
25772577
}
25782578

2579-
[TestCase("SetCash(decimal cash)")]
2580-
[TestCase("SetCash(string symbol, ...)")]
2581-
public void CanNotChangeAccountCurrencyAfterSettingCash(string overload)
2579+
[Test]
2580+
public void ChangeAccountCurrencyAfterImplicitSetCashRelabelsAmountToNewCurrency()
25822581
{
25832582
var algorithm = new QCAlgorithm();
2584-
if (overload == "SetCash(decimal cash)")
2585-
{
2586-
algorithm.Portfolio.SetCash(10);
2587-
}
2588-
else
2589-
{
2590-
algorithm.Portfolio.SetCash(Currencies.USD, 1, 1);
2591-
}
2592-
Assert.Throws<InvalidOperationException>(() => algorithm.Portfolio.SetAccountCurrency(Currencies.USD));
2583+
// SetCash(decimal) means "this many units of the (eventual) account currency",
2584+
// so switching the account currency re-labels the amount instead of preserving USD.
2585+
algorithm.Portfolio.SetCash(1);
2586+
2587+
Assert.DoesNotThrow(() => algorithm.Portfolio.SetAccountCurrency("BTC"));
2588+
2589+
Assert.AreEqual("BTC", algorithm.Portfolio.CashBook.AccountCurrency);
2590+
Assert.AreEqual(1m, algorithm.Portfolio.CashBook["BTC"].Amount);
2591+
Assert.IsFalse(algorithm.Portfolio.CashBook.ContainsKey(Currencies.USD));
2592+
}
2593+
2594+
[Test]
2595+
public void ChangeAccountCurrencyAfterExplicitSetCashKeepsPreviousCash()
2596+
{
2597+
var algorithm = new QCAlgorithm();
2598+
// SetCash(symbol, ...) commits cash to a specific currency, so switching the account
2599+
// currency must keep that balance in its own entry and start the new one at zero.
2600+
algorithm.Portfolio.SetCash(Currencies.USD, 100000, 1);
2601+
2602+
Assert.DoesNotThrow(() => algorithm.Portfolio.SetAccountCurrency("BTC"));
2603+
2604+
Assert.AreEqual("BTC", algorithm.Portfolio.CashBook.AccountCurrency);
2605+
Assert.AreEqual(100000m, algorithm.Portfolio.CashBook[Currencies.USD].Amount);
2606+
Assert.AreEqual(0m, algorithm.Portfolio.CashBook["BTC"].Amount);
2607+
}
2608+
2609+
[Test]
2610+
public void ChangeAccountCurrencyAfterImplicitSetCashAppliesStartingCashToNewCurrency()
2611+
{
2612+
var algorithm = new QCAlgorithm();
2613+
algorithm.Portfolio.SetCash(100000);
2614+
2615+
algorithm.Portfolio.SetAccountCurrency(Currencies.EUR, 50000);
2616+
2617+
Assert.AreEqual(Currencies.EUR, algorithm.Portfolio.CashBook.AccountCurrency);
2618+
Assert.AreEqual(50000m, algorithm.Portfolio.CashBook[Currencies.EUR].Amount);
2619+
Assert.IsFalse(algorithm.Portfolio.CashBook.ContainsKey(Currencies.USD));
2620+
}
2621+
2622+
[Test]
2623+
public void ChangeAccountCurrencyAfterExplicitSetCashAppliesStartingCashToNewCurrency()
2624+
{
2625+
var algorithm = new QCAlgorithm();
2626+
algorithm.Portfolio.SetCash(Currencies.USD, 100000, 1);
2627+
2628+
algorithm.Portfolio.SetAccountCurrency(Currencies.EUR, 50000);
2629+
2630+
Assert.AreEqual(Currencies.EUR, algorithm.Portfolio.CashBook.AccountCurrency);
2631+
Assert.AreEqual(100000m, algorithm.Portfolio.CashBook[Currencies.USD].Amount);
2632+
Assert.AreEqual(50000m, algorithm.Portfolio.CashBook[Currencies.EUR].Amount);
2633+
}
2634+
2635+
[Test]
2636+
public void SetAccountCurrencyWithSameCurrencyOverridesStartingCash()
2637+
{
2638+
var algorithm = new QCAlgorithm();
2639+
algorithm.Portfolio.SetCash(100000);
2640+
2641+
// Calling SetAccountCurrency with the same currency must overwrite the previously
2642+
// set cash amount instead of keeping the older value.
2643+
algorithm.Portfolio.SetAccountCurrency(Currencies.USD, 200000);
2644+
2645+
Assert.AreEqual(Currencies.USD, algorithm.Portfolio.CashBook.AccountCurrency);
2646+
Assert.AreEqual(200000m, algorithm.Portfolio.CashBook[Currencies.USD].Amount);
2647+
}
2648+
2649+
[Test]
2650+
public void SetAccountCurrencyWithSameCurrencyAndNoStartingCashKeepsExistingAmount()
2651+
{
2652+
var algorithm = new QCAlgorithm();
2653+
algorithm.Portfolio.SetCash(100000);
2654+
2655+
algorithm.Portfolio.SetAccountCurrency(Currencies.USD);
2656+
2657+
Assert.AreEqual(Currencies.USD, algorithm.Portfolio.CashBook.AccountCurrency);
2658+
Assert.AreEqual(100000m, algorithm.Portfolio.CashBook[Currencies.USD].Amount);
25932659
}
25942660

25952661
[Test]

0 commit comments

Comments
 (0)