Anupam Yadav
Anupam Yadav

Reputation: 4573

Design pattern to remove switch case

I have a requirement to verify whether the postal code for a particular country is mandatory or not based on the countryid supplied. Currently I'm doing this with a switch statement but this code breaks the Open/Closed SOLID principle. I would like to know how to get rid of the switch in this scenario.

public class PostCodeVerifyMandatory : IPostCodeVerifyMandatory {
    public bool IsPostCodeRequired(int countryId, string region)
    {
        switch (countryId) {
            case 1:     //UK
            case 12:    //Australia
            case 29:    //Brazil
            case 31:    //Brunei
            case 37:    //Canada
            case 56:    //Denmark
            case 105:   //Japan
            case 110:   //South Korea
            case 114:   //Latvia
            case 136:   //Moldova
            case 137:   //Monaco
            case 145:   //Netherlands
            case 165:   //Poland
            case 166:   //Portugal
            case 183:   //Slovak Republic (Slovakia)
            case 189:   //Spain
            case 196:   //Sweden
            case 197:   //Switzerland
            case 199:   //Taiwan Region
            case 213:   //Ukraine
            case 215:   //USA
            case 221:   //Vietnam
                return true;
            case 232:   //Ireland
                return region == "Dublin";
            default:
                return false;
        }
    }

}

Upvotes: 3

Views: 6413

Answers (4)

Tommy Grovnes
Tommy Grovnes

Reputation: 4156

Try this:

public class PostCodeVerifyMandatory : IPostCodeVerifyMandatory
{
    public List<Func<int, string, bool>> Rules { get; private set; }

    public PostCodeVerifyMandatory()
    {
        Rules = new List<Func<int, string, bool>>();
    }

    public bool IsPostCodeRequired(int countryId, string region)
    {
        if(Rules == null)
            return false;

        return (Rules.Any(r => r(countryId, region)));
    }
}

You will have to load the ruleset, before using it:

var simpleCountries = new List<int> 
                {
                    1,  // UK
                    12,  // Australia
                    29,  // Brazil
                    56,   // Brunei
                    //..
                    //..
                    215,   //USA
                    221   //Vietnam
                };

var postCodeVerifier = new PostCodeVerifyMandatory();

// 1 rule for simple countries
postCodeVerifier.Rules.Add((id, region) => simpleCountries.Contains(id)); 

// Special rule for Ireland
postCodeVerifier.Rules.Add((id, region) => id == 232 && region.Equals("Dublin")); 

var a = postCodeVerifier.IsPostCodeRequired(232, "Dublin");

or to make it fully data driven (using dictionary as example):

var countries = new Dictionary<int, string> 
                {
                    { 1, null },      // UK
                    { 12, null },     // Australia
                    { 29, null },     // Brazil
                    { 56, null },     // Brunei
                    //..
                    //..
                    { 215, null },    //USA
                    { 221, null },    //Vietnam
                    { 232, "Dublin" } // Ireland
                };

var postCodeVerifier = new PostCodeVerifyMandatory();

// 1 rule for all
postCodeVerifier.Rules.Add((id, region) => 
                              countries.ContainsKey(id) && 
                              (countries[id] ?? region) == region);

Upvotes: 1

guillaume31
guillaume31

Reputation: 14064

I'd probably follow that piece of advice from the c2 Wiki page "Switch statements smell" :

Using a database or TableOrientedProgramming is sometimes the appropriate "fix", not polymorphism. For example, store product classications are best handled in a database with many-to-many category tables, not case statements.

You could have something like :

public class Country 
{
  public List<Region> Regions { get; set; }

  public bool IsPostCodeRequiredByDefault { get; set; }
}

public class Region
{
  private bool? _isPostCodeRequired;

  public Country Country { get; set; }

  public bool IsPostCodeRequired 
  {
    get { return _isPostCodeRequired ?? Country.IsPostCodeRequiredByDefault; }
  }
}

Which also has the benefit of adressing a secondary "primitive obsession" smell by making region a first-class domain concept instead of just a string.

Upvotes: 1

Your switch statement effectively maps integers to booleans, with a default value of false.

So in this case, I would simply create a Dictionary<int,bool> with the appropriate values. Since the values are pretty much fixed, you could initialize it in the declaration:

Dictionary<int, bool> dict = new Dictionary<int, bool>() {
  {  1 /* UK */,        true  }, 
  { 12 /* Australia */, false } 
    ...etc...
};

As @Nick points out, the case for Ireland means you'll still need some extra logic, so you'll want the Dictionary to be private, and the answer accessible via your IsPostCodeRequired(int,strnig) method.

EDIT:
It would probably be best to get these values from a database, as @JustinHarvey points out.

If you want to be very strict about the Open/Closed Principle, you could use the Strategy design pattern - you would create a separate ConcreteStrategy object for every country. If a new country were added, you would create a new ConcreteStrategy object for that country. That way you can add the logic for countries with special rules, without touching your original code.
However, the number of countries with special rules is probably very small, so unless you really cannot change the code once it's in production, this is over-engineering.

Upvotes: 3

Vytalyi
Vytalyi

Reputation: 1695

Maybe something like this:

            private Dictionary<int, string> _dict;
            protected Dictionary<int, string> CountryDictionary
            {
                get
                {
                    if (_dict == null)
                    {
                        _dict = new Dictionary<int, string>();
                        _dict.Add(1, "UK");
                        _dict.Add(12, "Australia");
                        // and so on
                    }

                    return _dict;
                }
            }

            public class PostCodeVerifyMandatory : IPostCodeVerifyMandatory
            {
                public bool IsPostCodeRequired(int countryId, string region)
                {
                    return CountryDictionary.ContainsKey(countryId);
                }
            }

Upvotes: 1

Related Questions