Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

Imagine that I need a color palette form my Winforms application to have a consistent look.

What I did was create a static helper class, and helper methods that I can call from anywhere in my code and invoke what I need from the App.settings file.

Here for example, I getting the school name from the App.config file, so I can sell this application to other schools with minimal changes on my part.

Like so:

<?xml version="1.0" encoding="utf-8" ?>
<configuration>
  <appSettings>
    <add key="schoolName" value="Uboldi"/>
  </appSettings>
</configuration>


using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.Configuration;

namespace Uboldi.Helpers
{
    public static class CustomizationHelper
    {
        public static string GetSchoolName()
        {
            return ConfigurationManager.AppSettings["schoolName"];
        }
    }
}

Usage:

using System;
using System.Collections.Generic;
using System.ComponentModel;
using System.Data;
using System.Drawing;
using System.Linq;
using System.Text;
using System.Windows.Forms;
using Uboldi.Helpers;

namespace Uboldi
{
    public partial class MainForm : Form
    {
        public MainForm()
        {
            InitializeComponent();
            LoadFormTitle();
        }

        private void LoadFormTitle()
        {
            var schoolName = CustomizationHelper.GetSchoolName();
            this.Text = String.Format("Sistema {0} - Pagina Principal", schoolName);
        }
    }
}

Any glaring mistakes I'm making by choosing this type of architecture?

share|improve this question

8 Answers 8

up vote 23 down vote accepted

While somewhat of a tangent from your question but you may find it helpful nonetheless.

I would recommend looking at a custom ConfigurationSection which allows you to define more complex configuration hierarchies that are strongly-typed. I find it much easier to use a custom configuration section than having to remember a bunch of magic strings in the appSettings element and also allows you to specific which values are required, what the defaults values are, etc.

Using a custom configuration section you could create a configuration type like:

public class UboldiConfigurationSection : System.Configuration.ConfigurationSection {
    [ConfigurationProperty("schoolName")]
    public string SchoolName {
        get { return (string)this["schoolName"]; }
        set { this["schoolName"] = value; }
    }
}

Then to load that configuration type:

public static class UboldiApplcation {
    public static UboldiConfigurationSection Config { get; internal set; }

    public static void Initialize() {
        Config = ConfigurationManager.GetSection("uboldi") as UboldiConfigurationSection;
    }
}

The app.config then would look something like this:

<configuration>
    <configSections>
        <section name="uboldi" type="Uboldi.UboldiConfigurationSection, Uboldi" />
    </configSections>
    <uboldi schoolName="Fillmore Central" />
</configuration>

Lastly, you use the configuration by:

public void Test() {    
    //This only needs to be done once, presumably in your Program.Main method
    UboldiApplication.Initialize();

    var name = UboldiApplication.Config.SchoolName;
}

A couple of notes:

  1. You'll need to reference the System.Configuration assembly as it's not usually referenced in VS by default.
  2. The ConfigurationManager.GetSection("uboldi") is expecting the name of the section in the app.config file. You'll note that this matches in the example above.
  3. The section element in the app.config file uses the standard .Net type name convention to locate the specified configuration section. In this example I am assuming that the UboldiConfigurationSection type is the Uboldi namespace and in an Uboldi assembly (dll or exe).
  4. You can add hierarchy by creating ConfigurationElement sub classes and using them as properties in your configuration section and elements.
  5. The link above is for a Web.config, but the same thing is possible in an app.config file.
share|improve this answer

Is the purpose of CustomizationHelper just to abstract away the ConfigurationManager/AppConfig stuff? Because otherwise I'd just stick with the straight ConfigurationManager call.

The less hoops required to understand what's going on, the better in my book. Unless you see the need to someday get SchoolName from another source (like say a Database).

In any event the class name could probably be improved, maybe something like "SchoolConfiguration" and have a "Name" property or "GetName()" method.

share|improve this answer

I think that'd be fine. Another option would be to create a second constructor for the form and pass the title in:

public class MainForm : Form
{
    public MainForm()
    {
        InitializeComponent();
    }

    public MainForm(string schoolName) : this()
    {
        this.Text = String.Format("Sistema {0} - Pagina Principal", schoolName);
    }
}

This may be just personal preference, though.

You could also further extract the hardcoded form title into a config or resource file and instantiate the form like this instead:

var titleFormat = ...; // retrieve format from some place
var schoolName = CustomizationHelper.GetSchoolName();
var form = new MainForm() { Text = string.Format(titleFormat, schoolName) };

The payoff from doing that would depend on how likely it is that it'll change in the future or whether or not you're planning to translate the application into other languages.

share|improve this answer

Personally I would have made them static properties rather than static methods. Just seems more natural with C#. But the concept of having a static helper class to retrieve configuration values is a sound one. It becomes even more useful when you are retrieving non-string types since you can abstract away the casting to the helper class.

Just one comment - how about some error handling in the helper class to ensure that the configuration values are actually there?

share|improve this answer

The presented code seems okay, there aren't any obvious problems with it. CustomizationHelper could probably use a better name though to indicate what's being customized. If this is part of a larger project, putting a comment to indicate which classes are suppose to use CustomizationHelper would be a good idea.

share|improve this answer

I second the config section. It has the advantage to set a setting as required, throwing an exception at the GetSection call, instead of passing a null value with AppSettings[nonexistingKey]

share|improve this answer

A few thoughts:

  • The name "CustomizationHelper" is not very specific. How about CustomerBrandingService? Even though it "just" fetches data from a config file, that may not always be the case, and it is still an application service. (Naming classes with "Helper" is similar to naming with "Manager" - see reference on that below.)

Also, while your question is reasonable and simple, it is not clear to me what decisions you will make from this. For example, if this is the basis for a whole app, I suggest some other options to consider:

  • Why are you building a WinForm app in 2011? Consider WPF or Silverlight (possibly Silverlight Out-of-Browser "SLOOB").

  • If you choose WPF or Silverlight, the title would be assigned most naturally using Data Binding through the Model-View-ViewModel pattern.

Pointers for more information:

share|improve this answer

I think the appSettings section is a pretty neat solution for something as simple as your example, and I use this frequently over creating config sections, when no hierarchy is required. I do however find the following pattern useful to add consistency to the way appSettings are used, adding some typing, building in the idea of whether the setting is expected to be found in the .config file, and providing the ability to specify a default value.

public static class AppSettingsHelper
{
    private static TReturnType LoadAppSetting<TReturnType>(string name, bool required, TReturnType defaultValue)
    {
        // Check for missing settings
        if (!ArrayExt.Contains<string>(ConfigurationManager.AppSettings.AllKeys, name))
        {
            if (required)
                throw new ConfigurationErrorsException(string.Format("Required setting missing: {0}", name));
            else
                return defaultValue;
        }

        // Read the setting and return it
        AppSettingsReader reader = new AppSettingsReader();
        return (TReturnType)reader.GetValue(name, typeof(TReturnType));
    }

    //example boolean property
    public static bool IsSomethingSet
    {
        get
        {
            return ApplicationSettingsHelper.LoadAppSetting<bool>(
                "settingName",
                true,
                false);
        }
    }

    //int property
    public static int SomeCount
    {
        get
        {
            return ApplicationSettingsHelper.LoadAppSetting<int>(
                "someCount",
                true,
                0);
        }
    }
}

Use like this:

if (AppSettingsHelper.IsSomethingSet)
{
    Console.WriteLine(AppSettingsHelper.SomeCount);
}
share|improve this answer

protected by Jamal Dec 4 '13 at 3:34

Thank you for your interest in this question. Because it has attracted low-quality answers, posting an answer now requires 10 reputation on this site.

Would you like to answer one of these unanswered questions instead?