Click here to Skip to main content
11,715,165 members (63,329 online)
Click here to Skip to main content

Code Smells - Switch and If Statements

, 30 Nov 2013 CPOL 8K 22 5
Rate this:
Please Sign up or sign in to vote.
Refactoring code smell

Introduction

Some time ago, I found an interesting site with bad code examples. Code from this site shows bad practice in code writing, but does not correct it in the right way. And I decided to do it with some examples from this portal.

Initial Source Code

The initial source code was got from http://govnokod.ru/14088.

public static int GetCarMaxRoomNumberNominal(string trainName, TrainCar car)
{
    if (!String.IsNullOrEmpty(trainName))
    {
        trainName = trainName.ToUpper();
        if (trainName.StartsWith("САПСАН"))
        {
            return 66;
        }
        else if (trainName.StartsWith("ЛАСТОЧКА"))
        {
            return 103;
        }
        else if (trainName.StartsWith("АЛЛЕГРО"))
        {
            return 72;
        }
    }
    
    switch (car.Category)
    {
        case TrainCarCategory.ReservedSeat:
        case TrainCarCategory.Common:
            return 54;
        case TrainCarCategory.Compartment:
            return car.TwoStorey ? 112 : 36;
        case TrainCarCategory.Lux:
        case TrainCarCategory.Soft:
            return car.TwoStorey ? 96 : 18;
        case TrainCarCategory.Sedentary:
            if (car.ServiceClass.Contains("1C"))
            {
                return 42;
            }
            else if (car.ServiceClass.Contains("2C"))
            {
                return 80;
            }
            if (car.ServiceClass.Contains("3C"))
            {
                return 117;
            }
            return 1;
        default:
            return 1;
    }
}

Let's Begin

First of all, I've added the needed classes and enumerations. After that, I wrote unit tests for this method. And only then, I started to refactor it.

This example of code smells shows us the usage of switch statement. Switch in object oriented code is canonical code smell.

Classical refactoring for its code is usage dictionary instead of switch.

I created the following table for analysing relationship between dictionary keys and values.

As you can see, this key is complex and has a variable number of parts.

I created the following classes for dictionary's key:

GenericKey.cs

public class GenericKey<TParts> where TParts : new()
{
    #region Properties
 
    public TParts Parts { get; set; }
 
    #endregion Properties
 
    #region Constructor
 
    public GenericKey()
    {
        Parts = new TParts();
    }
 
    public GenericKey(TParts inItems)
    {
        Parts = inItems;
    }
 
    #endregion Constructor
    
    #region Public methods
 
    public override bool Equals(object obj)
    {
        return obj != null && Equals((GenericKey<TParts>)obj);
    }
 
    public bool Equals(GenericKey<TParts> anotherKey)
    {
        return anotherKey != null && Parts.Equals(anotherKey.Parts);
    }
 
    public override int GetHashCode()
    {
        return Parts.GetHashCode();
    }
    
    #endregion Public methods
} 

KeyParts.cs

public class KeyParts : IEnumerable
{
    #region Fields

    private readonly List<KeyPart> _items;
    private int _position;
    private const int PutAllInTheSameBasket = 0;

    #endregion Fields

    #region Property

    public int Count
    {
        get
        {
            return _items.Count;
        }
    }

    public KeyPart Current
    {
        get
        {
            return _items[_position];
        }
    }

    #endregion Property

    #region Constructor

    public KeyParts()
    {
        _items = new List<KeyPart>();
    }

    #endregion Constructor

    #region Private methods

    private static bool Compare(KeyParts parts1, KeyParts parts2)
    {
        return parts1.Count == parts2.Count
            && (
                from KeyPart key1Part in parts1
                select parts2.Cast<KeyPart>().Any(
                    key2Part => key1Part.Name == key2Part.Name
                    && key1Part.Value == key2Part.Value)
                ).All(part => part);
    }

    #endregion Private methods

    #region Public methods

    public KeyParts Add(KeyPartNames inName, object inValue)
    {
        return Add(new KeyPart(inName, inValue));
    }

    public KeyParts Add(KeyPart part)
    {
        if (part.IsValid)
        {
            _items.Add(part);
        }
        return this;
    }

    public override bool Equals(object obj)
    {
        return obj != null && Equals((KeyParts)obj);
    }

    public bool Equals(KeyParts anotherKey)
    {
        return anotherKey != null && Compare(this, anotherKey);
    }

    public override int GetHashCode()
    {
        return PutAllInTheSameBasket;
    }

    public IEnumerator GetEnumerator()
    {
        return _items.GetEnumerator();
    }

    public bool MoveNext()
    {
        _position++;
        return (_position < _items.Count());
    }

    public void Reset()
    {
        _position = 0;
    }

    #endregion Public methods
}

KeyPart.cs

public class KeyPart
{
    public KeyPartNames Name { get; private set; }
    public string Value { get; private set; }
    
    public bool IsValid
    {
        get { return !string.IsNullOrWhiteSpace(Value); }
    }
    
    private static string AsString(object value) 
    {
        if (value == null)
        {
            return string.Empty;
        }
        var enumValue = value as Enum;
        return enumValue != null ? enumValue.GetStringValue() : value.ToString();
    }
    
    public KeyPart(KeyPartNames inName, object inValue)
    {
        if (inValue != null && !(inValue is string) && 
        !(inValue is Enum) && !(inValue is bool))
        {
            throw new ArgumentException
            ("inValue may be only string, enum or bool");
        }
        Name = inName;
        Value = AsString(inValue);
    }
}

The initial source was refactored into this:

private static readonly Dictionary<GenericKey<KeyParts>, 
int> MaxRoomNumberNominals = new Dictionary<GenericKey<KeyParts>, int>
   {{ new GenericKey<KeyParts> { Parts = {{KeyPartNames.Category, Category.Sedentary}}}, 1},
    { new GenericKey<KeyParts> { Parts = {{KeyPartNames.Category, Category.None}}},1},
    { new GenericKey<KeyParts> { Parts = {{KeyPartNames.Name, Name.Peregrin}}}, 66},
    { new GenericKey<KeyParts> { Parts = {{KeyPartNames.Name, Name.Swallow}}}, 103},
    { new GenericKey<KeyParts> { Parts = {{KeyPartNames.Name, Name.Allegro}}}, 72},
    { new GenericKey<KeyParts> { Parts = {{KeyPartNames.Category, Category.ReservedSeat}}}, 54},
    { new GenericKey<KeyParts> { Parts = {{KeyPartNames.Category, Category.Common}}}, 54},
    { new GenericKey<KeyParts> { Parts = {{KeyPartNames.Category, Category.Compartment},
                                          {KeyPartNames.TwoStorey, true}}}, 112},
    { new GenericKey<KeyParts> { Parts = {{KeyPartNames.Category, Category.Compartment},
                                          {KeyPartNames.TwoStorey, false}}}, 36},
    { new GenericKey<KeyParts> { Parts = {{KeyPartNames.Category, Category.Lux},
                                          {KeyPartNames.TwoStorey, true}}}, 96},
    { new GenericKey<KeyParts> { Parts = {{KeyPartNames.Category, Category.Lux},
                                          {KeyPartNames.TwoStorey, false}}}, 18},
    { new GenericKey<KeyParts> { Parts = {{KeyPartNames.Category, Category.Soft},
                                          {KeyPartNames.TwoStorey, true}}}, 96},
    { new GenericKey<KeyParts> { Parts = {{KeyPartNames.Category, Category.Soft},
                                          {KeyPartNames.TwoStorey, false}}}, 18},
    { new GenericKey<KeyParts> { Parts = {{KeyPartNames.Category, Category.Sedentary},
                                          {KeyPartNames.Class, Service.First}}}, 42},
    { new GenericKey<KeyParts> { Parts = {{KeyPartNames.Category, Category.Sedentary},
                                          {KeyPartNames.Class, Service.Second}}}, 80},
    { new GenericKey<KeyParts> { Parts = {{KeyPartNames.Category, Category.Sedentary},
                                          {KeyPartNames.Class, Service.Third}}}, 117}};
                                          
/// <summary>
/// Source code after refactoring
/// </summary>
/// <param name="trainName"></param>
/// <param name="car"></param>
/// <returns></returns>
public static int GetCarMaxRoomNumberNominal(string trainName, TrainCar car)
{
    if (car == null) throw new ArgumentNullException("car");
    var key = RoomNumberKeyFactory.Create(trainName, car);
    return MaxRoomNumberNominals[key];
} 

History

  • November 30, 2013 - Initial release

License

This article, along with any associated source code and files, is licensed under The Code Project Open License (CPOL)

Share

About the Author

Dmitry Handin
Software Developer (Senior)
Russian Federation Russian Federation
No Biography provided

You may also be interested in...

Comments and Discussions

 
GeneralMy vote of 1 Pin
tyaramis1-Dec-13 20:40
membertyaramis1-Dec-13 20:40 
GeneralRe: My vote of 1 Pin
Dmitry Handin1-Dec-13 20:48
professionalDmitry Handin1-Dec-13 20:48 
GeneralRe: My vote of 1 Pin
zaphoed3-Dec-13 5:47
memberzaphoed3-Dec-13 5:47 
GeneralRe: My vote of 1 Pin
Dmitry Handin3-Dec-13 6:38
professionalDmitry Handin3-Dec-13 6:38 
GeneralRe: My vote of 1 Pin
zaphoed3-Dec-13 7:31
memberzaphoed3-Dec-13 7:31 
QuestionDifferent solution Pin
ervegter1-Dec-13 9:26
memberervegter1-Dec-13 9:26 
AnswerRe: Different solution Pin
Dmitry Handin1-Dec-13 9:54
memberDmitry Handin1-Dec-13 9:54 
GeneralRe: Different solution Pin
ervegter1-Dec-13 10:59
memberervegter1-Dec-13 10:59 
GeneralMy vote of 1 Pin
Ron Beyer30-Nov-13 12:26
professionalRon Beyer30-Nov-13 12:26 
GeneralMy vote of 1 Pin
Paulo Zemek30-Nov-13 11:11
professionalPaulo Zemek30-Nov-13 11:11 
GeneralMy vote of 1 Pin
abdurahman ibn hattab30-Nov-13 9:52
memberabdurahman ibn hattab30-Nov-13 9:52 
GeneralMy vote of 5 Pin
Volynsky Alex30-Nov-13 6:47
memberVolynsky Alex30-Nov-13 6:47 
GeneralRe: My vote of 5 Pin
Dmitry Handin1-Dec-13 21:22
professionalDmitry Handin1-Dec-13 21:22 
GeneralRe: My vote of 5 Pin
Volynsky Alex2-Dec-13 11:31
memberVolynsky Alex2-Dec-13 11:31 
SuggestionThoughts for improvements Pin
CHill6030-Nov-13 5:23
memberCHill6030-Nov-13 5:23 
GeneralRe: Thoughts for improvements Pin
Dmitry Handin1-Dec-13 21:35
professionalDmitry Handin1-Dec-13 21:35 

General General    News News    Suggestion Suggestion    Question Question    Bug Bug    Answer Answer    Joke Joke    Rant Rant    Admin Admin   

Use Ctrl+Left/Right to switch messages, Ctrl+Up/Down to switch threads, Ctrl+Shift+Left/Right to switch pages.

| Advertise | Privacy | Terms of Use | Mobile
Web04 | 2.8.150901.1 | Last Updated 30 Nov 2013
Article Copyright 2013 by Dmitry Handin
Everything else Copyright © CodeProject, 1999-2015
Layout: fixed | fluid