The multilingual duck So, we have this "Rubberduck Settings" window, with a number of "tabs" dedicated to each configurable feature. The "root" tab contains the application's "general settings" and, in the next release, we've found something to put there:



The display language! Credits to @SimonAndréForsberg and @Vogel612, for the Swedish and German translations respectively.. I did the French translation myself.

The first thing I tried after Simon's pull request was merged, was of course trying to switch the display language to Swedish... and that blew up with a CultureNotFoundException.

I had to handle the case when a culture wasn't supported/installed on the user's system, so this is how I went about the DisplayLanguageSetting class, which gets serialized into the application's xml configuration file:

namespace Rubberduck.Settings
{
    [XmlType(AnonymousType = true)]
    public class DisplayLanguageSetting
    {
        [XmlAttribute]
        public string Code { get; set; }

        public DisplayLanguageSetting()
        {
            // serialization constructor
        }

        public DisplayLanguageSetting(string code)
        {
            Code = code;

            CultureInfo culture;
            try
            {
                culture = CultureInfo.GetCultureInfo(code);
                _exists = true;
            }
            catch (CultureNotFoundException)
            {
                culture = RubberduckUI.Culture;
                _exists = false;
            }

            _name = RubberduckUI.ResourceManager.GetString("Language_" + Code.Substring(0, 2).ToUpper(), culture);
        }

        private readonly string _name;
        private readonly bool _exists;

        [XmlIgnore]
        public string Name { get { return _name; } }

        [XmlIgnore]
        public bool Exists { get { return _exists; } }

        public override bool Equals(object obj)
        {
            var other = (DisplayLanguageSetting) obj;
            return Code.Equals(other.Code);
        }

        public override int GetHashCode()
        {
            return Code.GetHashCode();
        }
    }
}




So the "general settings" tab is only loading the instances where the Exists getter is returning true. I'm particularly not fond of what I did in the LoadLanguageList method... would there be a better way?

namespace Rubberduck.UI.Settings
{
    public partial class GeneralSettingsControl : UserControl
    {
        private readonly IGeneralConfigService _configService;
        private readonly DisplayLanguageSetting _currentLanguage;

        public GeneralSettingsControl()
        {
            InitializeComponent();
            TitleLabel.Text = RubberduckUI.SettingsCaption_GeneralSettings;
            LanguageLabel.Text = RubberduckUI.Settings_LanguageLabel;

            LoadLanguageList();
        }

        public GeneralSettingsControl(DisplayLanguageSetting displayLanguage, IGeneralConfigService configService)
            : this()
        {
            _configService = configService;
            _currentLanguage = displayLanguage;
            LanguageList.SelectedItem = displayLanguage;
        }

        public DisplayLanguageSetting SelectedLanguage
        {
            get { return (DisplayLanguageSetting)LanguageList.SelectedItem; }
        }

        private void LoadLanguageList()
        {
            var settings = new[]
            {
                new DisplayLanguageSetting("en-US"),
                new DisplayLanguageSetting("fr-CA"),
                new DisplayLanguageSetting("sv-SV")
            };

            LanguageList.Items.AddRange(settings.Where(item =&gt; item.Exists).ToArray());
            LanguageList.DisplayMember = "Name";
        }
    }
}
I would group all of your properties together. Right now you have Code separated from the other properties by the ctors.
When overriding Equals, it's important to be sure that it can't throw an exception. The current implementation can throw an InvalidCastException.


    public override bool Equals(object obj)
    {
        var other = (DisplayLanguageSetting) obj;
        return Code.Equals(other.Code);
    }



A proper implementation would look something more like this.

public override bool Equals(object obj)
{
    var other = obj as DisplayLanguageSetting;

    if (other == null)
        return false;

    return Code.Equals(other.Code);
}

I don't much care for the control either.


 public partial class GeneralSettingsControl : UserControl



That's more or less all I need to see to know I don't care for it. How's that? Because that's all I need to know about the class to know that business logic is being put into the code behind instead of a presenter class. We've got a "smart" form on our hands here. Invert the control. This should be implementing an interface that exposes properties for the different view elements.

public interface IGeneralSettingsView
{
    DisplayLanguageSetting CurrentSetting {get; set;}
    IList&lt;DisplayLanguageSetting&gt; {get; set;}
}


Note that once you do this, the class no longer needs a Configuration Service at all. (The presenter will still need it though).