Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

When using NotSupportedException is bad?

I'm building a localization catalog and have a design dilemma. Right now, the catalog stores a Dictionary<string, IString> to store translations, where IString can be of two types: Singular or Plural. This is a simplified version of IString:

public interface IString
{
    void SetSingular(string singular);

    string GetSingular(params object[] args);

    void SetPlural(PluralCategory category, string plural);

    string GetPlural(PluralCategory category, params object[] args);
}

Then, when I implement Singular, I throw a NotSupportedException for the plural methods, which is caught by the catalog, and Plural does the same for the singular methods.

public class Singular : IString
{
    // ...

    public string GetPlural(PluralCategory category, params object[] args)
    {
        throw new NotSupportedException(string.Format(
            "Singular strings don't support GetPlural({0}, {1})",
            category, args));
    }

    public void SetPlural(PluralCategory category, string plural)
    {
        throw new NotSupportedException(string.Format(
            "Singular strings don't support SetPlural({0}, {1})",
            category, plural));
    }
}

This design comes from the requirement that keys must be unique for both singulars and plurals, but I feel that implementing non-related methods and throwing NotSupportedException doesn't smell well.

A second design would be to store a Dictionary<string, StringType> for key checking, where StringType is an enum, and then have two extra dictionaries Dictionary<string, Singular> and Dictionary<string, Plural> to be used depending on the value of StringType. This makes it a little more complex but doesn't break the interface concept.

So, what do you think? Is my first idea a bad use of NotSupportedException? Should I go for the second design or maybe something else?

Edit: a clearer solution, based on @dasblinkenlight idea, was to unify the interface methods to get a Singular or Plural into a single one. Singulars only have a PluralCategory.None, while Plurals are never allowed to contain this category. This is limited by the setter methods, left out of the interface to be defined by the specific implementations (the setter methods are not displayed below).

public interface IString
{
    string GetString(PluralCategory category, params object[] args);
    bool HasCategory(PluralCategory category);
}

public class Singular : IString
{
    private string Text;

    public string GetString(PluralCategory category, params object[] args)
    {
        if (category == PluralCategory.None)
        {
            if (Text == null || args == null || args.Length == 0)
            {
                return Text;
            }
            return string.Format(Text, args);
        }
        return null;
    }

    public bool HasCategory(PluralCategory category)
    {
        return category == PluralCategory.None;
    }
}

public class Plural : IString
{
    private Dictionary<PluralCategory, string> Texts = new Dictionary<PluralCategory, string>();

    public string GetString(PluralCategory category, params object[] args)
    {
        string text;
        if (Texts.TryGetValue(category, out text))
        {
            if (text == null || args == null || args.Length == 0)
            {
                return text;
            }
            return string.Format(text, args);
        }
        return null;
    }

    public bool HasCategory(PluralCategory category)
    {
        return Texts.ContainsKey(category);
    }
}
like image 499
moraes Avatar asked Mar 23 '23 22:03

moraes


1 Answers

You are right, this is not a good idea to throw NotSupportedException in a situation like that. Normally, you implement an interface is to provide a common set of operations that "unifies" a group of dissimilar classes providing different implementations for a set of common operations.

However, in your case the dissimilar classes fail to "unify": they retain not only their own implementations, but also their own interfaces. When you try using them uniformly, the code throws an exception. The common interface does not give you all that much compared to keeping them as object - users must know what class is behind the IString before making the call, otherwise they risk seeing a NotSupportedException.

There are several ways around this problem: one way would be unifying the singular and the plural methods, returning a more complex answer than a simple string:

public enum SingularOrPluralCategory {
    Singular,
    // The rest of the PluralCategory enum values
}
public class StringForm {
    public string Text {get; private set;}
    public SingularOrPluralCategory Category {get; private set;}
}
public interface IString {
    // You may want to omit the setter from the interface, adding it to the class instead
    void SetForm(SingularOrPluralCategory category, string plural);
    StringForm GetForm(SingularOrPluralCategory category, params object[] args);
}

Now the singular implementation will return StringForm with the Singular value in the Category, while the plural implementation would return StringForm with one of several plural categories. Trying to set a plural category on a singular subclass of IString may still be an error, but you can fix that by moving the setter onto the class, making it impossible to call a setter with a category on a singular subclass of IString.

like image 195
Sergey Kalinichenko Avatar answered Apr 04 '23 22:04

Sergey Kalinichenko