Need help solving "Switch Cases" Code Smell

Viewed 201

I'm in the middle of a refactoring but i'm confused about how to proceed, i will try to explain the problem.

The "code smell" i'm working on, is the amount of switch cases in the code. There are about fifteen switch cases that look similar to this:

foreach( var item in items)
{
    switch (item.Type)
    {
        case "Type 1":
            return Type1.GetMinPrice(); 
        break;
        
        case "Type 2":
            return Type2.GetMinPrice(); 
        break;
        
        case "Type 3":
            return Type3.GetMinPrice(); 
        break;
        
        case "Type 4":
            return Type4.GetMinPrice(); 
        break;
        ...
    }
}

The first thing i thought of doing was, i create an interface IType, then i let all the "Type" classes implement it... Something like this:

interface IType
{
    int GetMinPrice();
}

public class Type1:IType
{
    public int GetMinPrice()
    {
        return 1;
    }

}

public class Type2:IType
{
    public int GetMinPrice()
    {
        return 2;
    }

}
...

Then i move the switch in a new factory class that returns me the right instance, like:

public class TypeFactory()
{
    public static IType CreateType(string type)
    {
        switch (type)
        {
            case "Type 1":
                return new Type1();
            break;
            
            case "Type 2":
                return new Type2();
            break;
            
            case "Type 3":
                return new Type3();
            break;
            
            case "Type 4":
                return new Type4();
            break;
            ...
        }
    }
}

and finally i refactor the original method containing the switch case in something like this:

foreach( var item in items)
{
    IType rightType = TypeFactory.CreateType(item.Type);
    return rightType.GetMinPrice();
}

hurray i potentially reduced the number of switch cases with polymorphism.

Now the problem, what i found out is that the switch cases in the code are not always identical, with this i mean that in some situations, the switch contains all the cases, in other, it contains less cases ( simply because some types can/must default ).

To be more concrete, in the majority of the cases, some types need to return a string while other not, the implementation is something like this (actually also means that for other business logics, item of the wrong type never "arrive" into this switch):

switch (item.Type)
    {
        case "Type 1":
            return Type1.GetReminderText(); 
        break;
        
        case "Type 2":
            return Type2.GetReminderText();
        break;
        
        default:
            throw new Exception("Type not found");
        break;
        ...
    }

so i thought that a good idea would be to create a new interface, let's say IRemindable that is going to be implemented only in the Types which effectively use it... but in this case i'll need another factory to create the right concrete... so the original problem of reducing the amount of switch cases does not seem to be solved...i simply moved them around...

Clearly i could simply insert the GetReminderText() within the IType and force all the implementers to implement it (maybe doing nothing or returning an empty string) but this violates the INTERFACE SEGREGATION PRINCIPLE... I can't get my head around this... I also tried to take a look at the composition pattern to see if it could help but also there i was unable to find a proper solution, because, from what i understood, it works pretty well where i do not have a custom implementation of the "added" functionality but not that well if i've to write a custom implementation... to be more concrete:

if i add an ILogger to a class like this, it works so well it makes me cry:

public class Type1
{
    ILogger logger;
    public string GetReminderText()
    {
        logger.Log("retrieving reminder text .... ");
        ...
    }
}

but if i've to implement the GetReminderText within the added component, then it gets me confused... cause i would get something like

public class Type1
{
    IRemindable reminder;
    public string GetReminderText()
    {
        reminder.GetReminderText(); // <-- this needs to be a custom implementation for type1 this means that i've to create another factory... clearly getting out of hands..
    }
}

Can someone give me some suggestion on the right path to follow in these cases? Every kind of suggestion or consideration is very much appreciated.

Thanks for you time and have a nice day

1 Answers

If GetMinPrice() is a static method in each of the types then you can simply map the string to the static method:

private static Dictionary<string, Func<int>> mapping = new Dictionary<string, Func<int>>
{
    { "Type 1", Type1.GetMinPrice },
    // The other mappings here
};

From there:

if (!mapping.ContainsKey(item.Type))
    throw new Exception("Type is not mapped");

int minPrice = mapping[item.Type]();
Related