You have an enum or a switch statement - reidev275/SignsYouShouldProbablyRefactor GitHub Wiki
Example
public enum Color
{
Red, Blue
}
switch (color)
{
case Color.Red:
ProcessRed();
break;
case Color.Blue:
ProcessBlue();
break;
default:
throw new NotImplementedException(color);
}
Why is this bad?
- If we have already identified that code can change along a specific seam we should protect ourselves against future change along the seam to align with the Open/Closed principal.
- If we want to handle a new color we have to add code to process the color and also remember to update the switch statement which violates DRY principals.
Caveats
- Things like days of the week or months of the year that are collections of logic but will never change the items in the list. SOLID principals should, as always, still apply. If Thursday can change irrespective of Wednesday then we should still code to specific interfaces to protect ourselves from unnecessary change.
- Using an enum to avoid "stringly" typed code. Just be aware that the enum has a high probability of needing to be changed in this case.
Resolved
Several patterns work well to resolve the switch antipattern but they all involve a collection of handler type objects. In this case I chose a factory pattern, but Chain of Responsibility and Pipeline could also work well.
interface IColorHandler
{
void Process();
}
class ColorFactory
{
readonly IDictionary<string, IColorHandler> _colorHandlers;
public ColorFactory(IDictionary<string, IColorHandler> colorHandlers)
{
if (colorHandlers == null) throw new ArgumentNullException("colorHandlers");
_colorHandlers = colorHandlers;
}
public IColorHandler GetColorHanlder(string color)
{
return _colorHandlers.First(x => x.Key == color);
}
}