Handling CA - cywongg/2025 GitHub Wiki
Your code can benefit from several improvements to make it cleaner, more robust, and easier to maintain. Here's how you can address the issues:
Comparing floating-point numbers (like double
) directly using Equals(0.0)
can fail due to precision issues. Instead, you should use a tolerance-based comparison to check if a double
is close enough to zero:
bool IsZero(double value, double tolerance = 1e-10)
{
return Math.Abs(value) < tolerance;
}
You can then replace:
fee.HasValue && fee.Equals(0.0)
with:
fee.HasValue && IsZero(fee.Value)
This ensures that even minor floating-point inaccuracies won't cause the comparison to fail.
You mentioned that you'll be repeating similar logic in other functions. To reduce duplication and improve readability, you should extract reusable methods.
Encapsulate the logic for getting basketInstrumentIds
into its own function:
private ICollection<int> GetBasketInstrumentIds(string udleLotCode, DateTime fixingDate, DateTime targetFixingDate)
{
var instruments = MainWindow.GetInstrumentByUdLELot(udleLotCode).ToList();
var maxFixingDate = instruments.Select(instr => instr.fixingDateNext).Max();
return fixingDate.Equals(targetFixingDate)
? instruments.Select(instr => instr.basketInstrumentID).ToList()
: null;
}
Use this method wherever needed:
basketInstrumentIds = GetBasketInstrumentIds(pdiKey.Item1, fixingDate, pdiKey.Item2);
Encapsulate the logic for calculating updated fees into a function:
private int CalculateUpdatedFees(List<int?> factorIndexHistoryIds, int feeType)
{
var factorIndexHistoryFees = FbiConnection.getFactorIndexHistoryFee(factorIndexHistoryIds, feeType);
return factorIndexHistoryFees
.Where(fee => fee.HasValue && IsZero(fee.Value))
.Count();
}
Use it like this:
int updatedFees = CalculateUpdatedFees(factorIndexHistoryIds, feeType);
Elastic.addLog($"Number of fees updated: {updatedFees}");
Encapsulate the repeated creation of CompositeCorporateAction
objects into a factory method:
private CompositeCorporateAction CreateCompositeCorporateAction(
Tuple<string, DateTime> pdiKey,
ICorporateAction alcorPdi,
ICorporateAction qDataPdi,
ICollection<int> basketInstrumentIds,
int updatedFees)
{
return new CompositeCorporateAction
{
UdLELiotCode = pdiKey.Item1,
ExerciseDate = pdiKey.Item2,
AlcorDivGross = alcorPdi?.DivGross ?? double.NaN,
AlcorDivNet = alcorPdi?.DivNet ?? double.NaN,
AlcorShivaDivNet = alcorPdi?.ShivaDivNet ?? double.NaN,
AlcorRFactor = alcorPdi?.RFactor ?? double.NaN,
QdataDivGross = qDataPdi?.DivGross ?? double.NaN,
QdataDivNet = qDataPdi?.DivNet ?? double.NaN,
QdataRFactor = qDataPdi?.RFactor ?? double.NaN,
TotalProducts = basketInstrumentIds?.Count.ToString() ?? "0",
Status = $"{updatedFees} / {basketInstrumentIds?.Count ?? 0}"
};
}
Then use it like this:
var mappedPdi = CreateCompositeCorporateAction(pdiKey, alcorPdi, qDataPdi, basketInstrumentIds, updatedFees);
App.Current.Dispatcher.Invoke(() => _pdis.Add(mappedPdi));
Since _pdis
is an ObservableCollection
, it automatically supports UI updates when items are added, removed, or modified. To update the Status
field asynchronously:
Find the CompositeCorporateAction
object in _pdis
and update its Status
property. The UI will refresh automatically:
private async Task UpdatePdiStatusAsync(string udleLotCode, DateTime exerciseDate, int updatedFees)
{
var pdiToUpdate = _pdis.FirstOrDefault(pdi =>
pdi.UdLELiotCode == udleLotCode && pdi.ExerciseDate == exerciseDate);
if (pdiToUpdate != null)
{
await Task.Run(() =>
{
pdiToUpdate.Status = $"{updatedFees} / {pdiToUpdate.TotalProducts}";
});
}
}
Alternatively, replace the object in _pdis
:
private async Task ReplacePdiAsync(CompositeCorporateAction updatedPdi)
{
await Task.Run(() =>
{
var index = _pdis.IndexOf(_pdis.FirstOrDefault(pdi =>
pdi.UdLELiotCode == updatedPdi.UdLELiotCode &&
pdi.ExerciseDate == updatedPdi.ExerciseDate));
if (index >= 0)
{
App.Current.Dispatcher.Invoke(() =>
{
_pdis[index] = updatedPdi;
});
}
});
}
The foreach
loop logic can be refactored into a function:
private void ProcessCorporateActions(IEnumerable<Tuple<string, DateTime>> pdiKeys)
{
foreach (var pdiKey in pdiKeys)
{
alcorPdis.TryGetValue(pdiKey, out var alcorPdi);
qDataPdis.TryGetValue(pdiKey, out var qDataPdi);
var basketInstrumentIds = GetBasketInstrumentIds(pdiKey.Item1, fixingDate, pdiKey.Item2);
var basketChangeIds = basketInstrumentIds?
.SelectMany(id => factorDataSetsByBasketInstrumentId.TryGetValue(id, out var values)
? values.Where(value => value != null).Select(value => value.changeID)
: Enumerable.Empty<int>())
.ToList();
var factorIndexHistoryIds = FbiConnection.getHistoryFeelds(basketChangeIds);
int updatedFees = CalculateUpdatedFees(factorIndexHistoryIds, 13);
var mappedPdi = CreateCompositeCorporateAction(pdiKey, alcorPdi, qDataPdi, basketInstrumentIds, updatedFees);
App.Current.Dispatcher.Invoke(() => _pdis.Add(mappedPdi));
}
}
Then call it like this:
ProcessCorporateActions(alcorPdis.Keys.Union(qDataPdis.Keys));
-
Use meaningful variable names: For example, rename
InstrunentsImpacted
toimpactedInstruments
andfactorIndexHistoryFees
tofees
. - Use LINQ carefully: Avoid chaining too many LINQ methods in one line to improve readability.
-
Avoid inline object initialization: Move initialization of complex objects (like
CompositeCorporateAction
) into separate methods or builders.
Following these recommendations will make your code more modular, testable, and easier to maintain. Extracting logic into reusable methods is the key to adhering to clean code principles.
fee.Equals(0.0)
can fail due to floating-point precision issues. In .NET, double
values are represented as IEEE 754 floating-point numbers, which cannot precisely represent all decimal values. As a result:
- Arithmetic operations on
double
values can introduce small rounding errors. - Even if you expect a value to be exactly
0.0
, it might instead be something like1e-15
or-1e-15
.
For example:
double fee = 0.1 - 0.1; // You may expect this to be 0.0
Console.WriteLine(fee.Equals(0.0)); // This might return false, depending on precision
Instead of using fee.Equals(0.0)
, compare floating-point numbers with a small tolerance (also known as an epsilon):
bool IsZero(double value, double tolerance = 1e-10)
{
return Math.Abs(value) < tolerance;
}
// Example usage:
if (fee.HasValue && IsZero(fee.Value))
{
// Fee is considered zero
}
This approach accounts for minor rounding errors and ensures a more robust comparison.
To handle the conditional initialization of CompositeCorporateAction
depending on whether fixingDate.Equals(pdiKey.Item2)
or not, the best approach is to separate the logic into a factory method or builder that handles each case.
private CompositeCorporateAction CreateCompositeCorporateAction(
Tuple<string, DateTime> pdiKey,
ICorporateAction alcorPdi,
ICorporateAction qDataPdi,
ICollection<int> basketInstrumentIds,
int updatedFees,
bool hasFixingDateMatch)
{
if (hasFixingDateMatch)
{
return new CompositeCorporateAction
{
UdLELiotCode = pdiKey.Item1,
ExerciseDate = pdiKey.Item2,
AlcorDivGross = alcorPdi?.DivGross ?? double.NaN,
AlcorDivNet = alcorPdi?.DivNet ?? double.NaN,
AlcorShivaDivNet = alcorPdi?.ShivaDivNet ?? double.NaN,
AlcorRFactor = alcorPdi?.RFactor ?? double.NaN,
QdataDivGross = qDataPdi?.DivGross ?? double.NaN,
QdataDivNet = qDataPdi?.DivNet ?? double.NaN,
QdataRFactor = qDataPdi?.RFactor ?? double.NaN,
TotalProducts = basketInstrumentIds?.Count.ToString() ?? "0",
Status = $"{updatedFees} / {basketInstrumentIds?.Count ?? 0}"
};
}
else
{
return new CompositeCorporateAction
{
UdLELiotCode = pdiKey.Item1,
ExerciseDate = pdiKey.Item2,
AlcorDivGross = alcorPdi?.DivGross ?? double.NaN,
AlcorDivNet = alcorPdi?.DivNet ?? double.NaN,
AlcorShivaDivNet = alcorPdi?.ShivaDivNet ?? double.NaN,
AlcorRFactor = alcorPdi?.RFactor ?? double.NaN,
QdataDivGross = qDataPdi?.DivGross ?? double.NaN,
QdataDivNet = qDataPdi?.DivNet ?? double.NaN,
QdataRFactor = qDataPdi?.RFactor ?? double.NaN
};
}
}
When creating the CompositeCorporateAction
, pass a flag indicating whether the fixingDate
matches:
bool hasFixingDateMatch = fixingDate.Equals(pdiKey.Item2);
var mappedPdi = CreateCompositeCorporateAction(
pdiKey,
alcorPdi,
qDataPdi,
basketInstrumentIds,
updatedFees,
hasFixingDateMatch
);
App.Current.Dispatcher.Invoke(() => _pdis.Add(mappedPdi));
This way, you separate the logic for deciding which fields to initialize into a single reusable method.
If you expect all fixingDateNext
values in instrumentsImpacted
to be the same, using Max()
is fine because it will return the same value for all elements. However, depending on the situation, it may be better to ensure that this assumption holds true (e.g., by verifying the uniformity of fixingDateNext
).
If you want to validate your assumption, you can check if all values are identical before using Max()
:
var fixingDates = instrumentsImpacted.Select(instrument => instrument.fixingDateNext).Distinct().ToList();
if (fixingDates.Count > 1)
{
throw new InvalidOperationException("Inconsistent fixing dates found in instruments.");
}
DateTime fixingDate = fixingDates.First();
This ensures that you only proceed if all fixingDateNext
values are consistent. If they are not, it throws an exception, making debugging easier.
Putting it all together:
private ICollection<int> GetBasketInstrumentIds(string udleLotCode, DateTime fixingDate, DateTime targetFixingDate)
{
var instruments = MainWindow.GetInstrumentByUdLELot(udleLotCode).ToList();
var maxFixingDate = instruments.Select(instr => instr.fixingDateNext).Max();
return fixingDate.Equals(targetFixingDate)
? instruments.Select(instr => instr.basketInstrumentID).ToList()
: null;
}
private CompositeCorporateAction CreateCompositeCorporateAction(
Tuple<string, DateTime> pdiKey,
ICorporateAction alcorPdi,
ICorporateAction qDataPdi,
ICollection<int> basketInstrumentIds,
int updatedFees,
bool hasFixingDateMatch)
{
if (hasFixingDateMatch)
{
return new CompositeCorporateAction
{
UdLELiotCode = pdiKey.Item1,
ExerciseDate = pdiKey.Item2,
AlcorDivGross = alcorPdi?.DivGross ?? double.NaN,
AlcorDivNet = alcorPdi?.DivNet ?? double.NaN,
AlcorShivaDivNet = alcorPdi?.ShivaDivNet ?? double.NaN,
AlcorRFactor = alcorPdi?.RFactor ?? double.NaN,
QdataDivGross = qDataPdi?.DivGross ?? double.NaN,
QdataDivNet = qDataPdi?.DivNet ?? double.NaN,
QdataRFactor = qDataPdi?.RFactor ?? double.NaN,
TotalProducts = basketInstrumentIds?.Count.ToString() ?? "0",
Status = $"{updatedFees} / {basketInstrumentIds?.Count ?? 0}"
};
}
else
{
return new CompositeCorporateAction
{
UdLELiotCode = pdiKey.Item1,
ExerciseDate = pdiKey.Item2,
AlcorDivGross = alcorPdi?.DivGross ?? double.NaN,
AlcorDivNet = alcorPdi?.DivNet ?? double.NaN,
AlcorShivaDivNet = alcorPdi?.ShivaDivNet ?? double.NaN,
AlcorRFactor = alcorPdi?.RFactor ?? double.NaN,
QdataDivGross = qDataPdi?.DivGross ?? double.NaN,
QdataDivNet = qDataPdi?.DivNet ?? double.NaN,
QdataRFactor = qDataPdi?.RFactor ?? double.NaN
};
}
}
private void ProcessCorporateActions(IEnumerable<Tuple<string, DateTime>> pdiKeys)
{
foreach (var pdiKey in pdiKeys)
{
alcorPdis.TryGetValue(pdiKey, out var alcorPdi);
qDataPdis.TryGetValue(pdiKey, out var qDataPdi);
var basketInstrumentIds = GetBasketInstrumentIds(pdiKey.Item1, fixingDate, pdiKey.Item2);
var basketChangeIds = basketInstrumentIds?
.SelectMany(id => factorDataSetsByBasketInstrumentId.TryGetValue(id, out var values)
? values.Where(value => value != null).Select(value => value.changeID)
: Enumerable.Empty<int>())
.ToList();
var factorIndexHistoryIds = FbiConnection.getHistoryFeelds(basketChangeIds);
int updatedFees = CalculateUpdatedFees(factorIndexHistoryIds, 13);
bool hasFixingDateMatch = fixingDate.Equals(pdiKey.Item2);
var mappedPdi = CreateCompositeCorporateAction(pdiKey, alcorPdi, qDataPdi, basketInstrumentIds, updatedFees, hasFixingDateMatch);
App.Current.Dispatcher.Invoke(() => _pdis.Add(mappedPdi));
}
}
This approach ensures clarity, modularity, and robustness. You can easily test each method individually and adapt them as needed.