1
\$\begingroup\$

I have a basic class that I'd like to get some feedback on. I'm refactoring some old code but the new code looks more complex. Basically I'm passing a list of included and excluded cultures into a function that should return if the current culture should be allowed. Exclusions win. Any advice how to better it would be great. Thanks

public static class CultureCheck
{
    public static bool IsCurrentCultureIncluded(string includedCultures, string excludedCultures)
    {
        if(includedCultures == null)
        {
            throw new ArgumentNullException("includedCultures");
        }

        if (excludedCultures == null)
        {
            throw new ArgumentNullException("excludedCultures");
        }

        var isIncluded = false;

        // exclusions win.
        if (IsCultureInList(excludedCultures) == true)
        {
            isIncluded = false;
        }
        else if(IsCultureInList(includedCultures) == true)
        {
            isIncluded = true;
        }

        return isIncluded;
    }

    private static bool IsCultureInList(string cultures)
    {
        string currentCultureName = Thread.CurrentThread.CurrentCulture.Name;
        var isFound = false;

        if (cultures.IndexOf("*") != -1)
        {
            isFound = true;
        }
        else
        {
            List<string> cultureList = new List<string>(cultures.Split(','));
            if (cultureList.Where(e => e.Equals(currentCultureName, StringComparison.InvariantCultureIgnoreCase)).Any() == true)
            {
                isFound = true;
            }
        }

        return isFound;
    }
}

OLD CODE

 public static bool IsCurrentCultureIncluded(string includedCultures, string excludedCultures)
    {
        #region Exclusion Test

        // exclusions win.
        if (excludedCultures.IndexOf("*") != -1)
        {
            return false;
        }

        string currentCultureName = Thread.CurrentThread.CurrentCulture.Name;
        string[] excludedCulturesSplit = excludedCultures.Split(new char[] { ',' });
        foreach (string excludedCulture in excludedCulturesSplit)
        {
            if (string.Equals(currentCultureName, excludedCulture, StringComparison.InvariantCultureIgnoreCase))
            {
                return false;
            }
        }

        #endregion

        #region Inclusion Test

        if (includedCultures.IndexOf("*") != -1)
        {
            return true;
        }

        string[] includedCulturesSplit = includedCultures.Split(new char[] { ',' });
        foreach (string includedCulture in includedCulturesSplit)
        {
            if (string.Equals(currentCultureName, includedCulture, StringComparison.InvariantCultureIgnoreCase))
            {
                return true;
            }
        }

        #endregion

        return false;
    }
\$\endgroup\$
2
\$\begingroup\$

I would do it like this (WARNING - not tested):

public static void SampleUsage()
{
    IsCurrentCultureIncluded("*", "*", Thread.CurrentThread.CurrentCulture.Name);
}

public static bool IsCurrentCultureIncluded(string includedCultures, string excludedCultures, string currentCultureName)
{
    return !CheckIncl(currentCultureName, excludedCultures) &&
            CheckIncl(currentCultureName, includedCultures);
}

private static bool CheckIncl(string currentCultureName, string includedCultures)
{
    if (includedCultures.IndexOf("*") != -1)
        return true;

    var includedCulturesSplit = includedCultures.Split(new[] { ',' });
    return includedCulturesSplit.Any(includedCulture => string.Equals(currentCultureName, includedCulture, StringComparison.InvariantCultureIgnoreCase));
}
\$\endgroup\$
  • \$\begingroup\$ You're missing the check for null, in CheckIncl, so it's not the same. Otherwise, it looks perfect to me. \$\endgroup\$ – ANeves Apr 25 '12 at 17:29

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.