7
\$\begingroup\$

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.

\$\endgroup\$
  • \$\begingroup\$ Per @Carl Manaster s answer: using a table is simpler in this case than using a switch or a dictionary. Table is more natural. The details are up to you - you can store this mapping in the database, in an Excel file, in a CSV file, and then run LINQ on it ... \$\endgroup\$ – Leonid Mar 13 '12 at 20:44
5
\$\begingroup\$

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;
}
\$\endgroup\$
  • \$\begingroup\$ The basic idea is sound, simple to maintain (adding languages or types), and it is very compact. I can translate this easily into C#. Thank you! \$\endgroup\$ – dotancohen Mar 16 '12 at 22:53
6
\$\begingroup\$

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();
}
\$\endgroup\$
  • 1
    \$\begingroup\$ Instead of the conditional expression after return, you could also put each language dictionary in a dictionary with lang as key too. Dictionary<string, Dictionary<string, string>> {{ "heb", new Dictionar<string, string> {{ "עובד", "employee" }, ...}}, "ENG", new Dictionary<string, string> {{ "supplier", "SUPPLIER" }, ...}}; :) \$\endgroup\$ – Lars-Erik Mar 13 '12 at 13:50
2
\$\begingroup\$

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.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.