Refactor embedded switch

Question

A particular application may refer to a specific property in one of three languages in different contexts: in lowercase English, in uppercase English, and in Hebrew. To facilitate ease of converting one language to another, this monstrosity exists:

public string translatePersonType(string personType, string lang)
{

    switch (personType)
    {
        case "employee":
        case "EMPLOYEE":
        case "עובד":
            switch (lang)
            {
                case "eng":
                    return "employee";
                case "ENG":
                    return "EMPLOYEE";
                case "heb":
                    return "עובד";
            }
            break;

        case "contractor":
        case "CONTRACTOR":
        case "קבלן":
            switch (lang)
            {
                case "eng":
                    return "contractor";
                case "ENG":
                    return "CONTRACTOR";
                case "heb":
                    return "קבלן";
            }
            break;

        case "supplier":
        case "SUPPLIER":
        case "ספק":
            switch (lang)
            {
                case "eng":
                    return "supplier";
                case "ENG":
                    return "SUPPLIER";
                case "heb":
                    return "ספק";
            }
            break;

        case "customer":
        case "CUSTOMER":
        case "לקוח":
            switch (lang)
            {
                case "eng":
                    return "customer";
                case "ENG":
                    return "CUSTOMER";
                case "heb":
                    return "לקוח";
            }
            break;

        default:
            return "";

    }// end switch (personType)

    return "";

}// end translatePersonType

Can this be refactored to be more concise and maintainable? For reference, this is the database field behind the property:

personType SET('CUSTOMER','SUPPLIER','EMPLOYEE', 'CONTRACTOR') NOT NULL

Thanks.

Answer 1

OK, this is Java and might take a little tickling to work in C#, but:

final String[] employee = {"employee", "EMPLOYEE", "עובד"};
final String[] contractor = {"contractor", "CONTRACTOR", "קבלן"};
final String[] supplier = {"supplier", "SUPPLIER", "ספק"};
final String[] customer = {"customer", "CUSTOMER", "לקוח"};
final String[][] classifications = {employee, contractor, supplier, customer};

public String translatePersonType(String personType, String lang) {
    int index = 0;
    if (lang == "ENG") index = 1;
    if (lang == "heb") index = 2;
    for (String[] classification : classifications) {
        if (Arrays.asList(classification).contains(personType)) {
            return classification[index];
        }
    }
    return null;
}

Answer 2

This kind of switch statements can be refactored using maps/dictionaries. I would do it like this:

Edit: I actually made a couple of mistakes. After adding more unit tests I came up with a hopefully better version.

//V. 2:
public static string TranslatePersonTypeBetterRefactored(string personType, string lang)
{
    var allDicts = new Dictionary<string, Dictionary<string, string>>
                        {
                            {
                                "heb", new Dictionary<string, string>()
                                            {
                                                { "employee", "עובד" },
                                                { "contractor", "קבלן" },
                                                { "supplier", "ספק" },
                                                { "customer", "לקוח" }
                                            }
                            },
                            {
                                "eng", new Dictionary<string, string>()
                                            {
                                                { "עובד", "employee" },
                                                { "קבלן", "contractor" },
                                                { "ספק", "supplier" },
                                                { "לקוח", "customer" }
                                            }
                            },
                            {
                                "_eng", new Dictionary<string, string>()
                                            {
                                                { "employee", "employee" },
                                                { "contractor", "contractor" },
                                                { "supplier", "supplier" },
                                                { "customer", "customer" }
                                            }
                            }
                        };

    Dictionary<string, string> dict;
    string tran;

    if ((allDicts.TryGetValue(lang.ToLower(), out dict) &&
         dict.TryGetValue(personType.ToLower(), out tran)) ||
        (allDicts.TryGetValue(string.Format("_{0}", lang.ToLower()), out dict) &&
         dict.TryGetValue(personType.ToLower(), out tran)))
    {
        var shouldBeUpper = lang.Any(c => char.IsUpper(c));

        return shouldBeUpper ? tran.ToUpper()
                             : tran;
    }

    return string.Empty;
}

//V. 1 (buggy):
public static string TranslatePersonTypeRefactored(string personType, string lang)
{
    var engToUpper = lang == "ENG";
    var engToLower = lang == "eng";

    if (engToUpper && personType.Any(c => char.IsLower(c)))
        return personType.ToUpper();

    if (engToLower && personType.Any(c => char.IsUpper(c)))
        return personType.ToLower();

    var hebrewToEnglishWords = new Dictionary<string, string>
                        {
                            { "עובד", "employee" },
                            { "קבלן", "contractor" },
                            { "ספק", "supplier" },
                            { "לקוח", "customer" }
                        };

    var hebrewToEngTranslation = hebrewToEnglishWords[personType];

    return engToLower ? hebrewToEngTranslation
                      : hebrewToEngTranslation.ToUpper();
}

Answer 3

I think something like this should be enclosed in a class:

class PersonType
{
    private readonly string m_englishName;
    private readonly string m_hebrewName;

    public string EnglishNameLowerCase { get { return m_englishName.ToLower(); } }
    public string EnglishNameUpperCase { get { return m_englishName.ToUpper(); } }
    public string HebrewName { get { return m_hebrewName; } }

    public PersonType(string englishName, string hebrewName)
    {
        m_englishName = englishName;
        m_hebrewName = hebrewName;
    }

    public bool IsMatch(string name)
    {
        return name == EnglishNameLowerCase
            || name == EnglishNameUpperCase
            || name == HebrewName;
    }

    public string GetName(string lang)
    {
        switch (lang)
        {
            case "eng":
                return EnglishNameLowerCase;
            case "ENG":
                return EnglishNameUpperCase;
            case "heb":
                return HebrewName;
        }

        throw new ArgumentException("lang");
    }
}

…

private static readonly PersonType[] PersonTypes =
    new[]
    {
        new PersonType("employee", "עובד"),
        new PersonType("contractor", "קבלן"),
        new PersonType("supplier", "ספק"),
        new PersonType("customer", "לקוח")
    }

public string TranslatePersonType(string personType, string lang)
{
    return PersonTypes.Single(pt => pt.IsMatch(personType).GetName(lang);
}

Even though it's still quite long, it would be much easier to add another person type or language.

Also, I think it would make more sense if lang was an enum, not a string.