Given:
internal void Configure(ButtonEventArgs args, IBroker broker, FunctionEntry entry)
{
int phase = broker.TradingPhase;
if (args.Button == ItemType.SendAutoButton)
{
if (phase == 1)
{
entry.SetParameter("ANDealerPrice", -1);
entry.SetParameter("ANAutoUpdate", 4);
}
else if (phase == 2)
{
entry.SetParameter("ANDealerPrice", -1);
entry.SetParameter("ANAutoUpdate", 2);
}
}
if (phase == 1)
{
if (broker.IsCashBMK)
{
entry.SetParameter("Value", 100);
}
else if (broker.IsCross)
{
entry.SetParameter("Value", 200);
}
}
}
I am looking for suggestions to refactor the code above. As suggested by Fowler: "Replace condition with Strategy/polymorphism", i fail to implement an effective code on these line. Since there are multiple conditions, base on mutiple variables.
Please suggest if there could be a pattern that could eliminate these error-prone and ugly conditions (code smell).
Thanks for your interest.
Edit: 1) My intention is to apply Open-Closed principle here, so that if tommorrow if there is a change in logic i can extend these conditions by introducing a new class. 2) Please don't mind the magic numbers, in real scenario i have valid constant/source for them.
For what you've presented so far, I would be inclined to separate out the three parameters, each into its own function, thus:
void SetAnDealerPrice(ButtonEventArgs args, IBroker broker,
FunctionEntry entry) {
if (args.Button != ItemType.SendAutoButton)
return;
int phase = broker.TradingPhase;
if (phase == 1 || phase == 2)
entry.SetParameter("ANDealerPrice", -1);
}
void SetAnAutoUpdate(ButtonEventArgs args, IBroker broker,
FunctionEntry entry) {
if (args.Button != ItemType.SendAutoButton)
return;
switch (broker.TradingPhase) {
case 1:
entry.SetParameter("ANAutoUpdate", 4);
break;
case 2:
entry.SetParameter("ANAutoUpdate", 2);
break;
}
}
void SetValue(IBroker broker, FunctionEntry entry) {
if (broker.TradingPhase != 1)
return;
entry.SetParameter("Value", broker.IsCashBMK ? 100 : 200);
}
This is somewhat hand-crafted (doesn't lend itself well to semi-automated updating as rules change), and marginally less efficient (some conditions are being checked, and some fields being referenced, multiple times, plus of course it's more function calls). I don't think efficiency matters until you have a demonstrated problem (and when you do, bigger changes than these will be required), and this approach leaves you knowing exactly what code to look at when the rules for a given parameter change. I don't believe polymorphism is likely to lead you to a good solution here.
If you love us? You can donate to us via Paypal or buy me a coffee so we can maintain and grow! Thank you!
Donate Us With