Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Refactor my code: Conditions based on different variables

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.

like image 817
Manish Basantani Avatar asked Jan 25 '26 05:01

Manish Basantani


1 Answers

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.

like image 95
Carl Manaster Avatar answered Jan 27 '26 18:01

Carl Manaster