Application Settings Helper

Question

I'm developing an app which stores some different user settings in app.config such as connection strings, paths for saving reports, default printer e.t.c For retrieving them I made a static class StoredSettingsTools which contains an Enum of settings types and a public method which calls (depending of setting type) some private method and get the settings from app.config:

public static class StoredSettingsTools
{

    public enum StoredSettingsType
    {
        DBConnection,
        Printer,
        ReportsFolder
    };

    private static ConnectionStringsSection GetConfigurationConnectionStringsSection()
    {
        ExeConfigurationFileMap configMap = new ExeConfigurationFileMap();
        Configuration config = ConfigurationManager.OpenExeConfiguration(System.Reflection.Assembly.GetEntryAssembly().Location);
        ConnectionStringsSection conectionSettings = (ConnectionStringsSection)config.SectionGroups.Get("UserSettingsGroup")
            .SectionGroups.Get("StoredConnectionSettings")
            .Sections.Get("defaultConnectionString");        
        return conectionSettings;
    }

    private static Hashtable GetConfigurationPrinterSection()
    {
        ExeConfigurationFileMap configMap = new ExeConfigurationFileMap();
        Configuration config = ConfigurationManager.OpenExeConfiguration(System.Reflection.Assembly.GetEntryAssembly().Location);
        ConfigurationSectionGroup userSettingsGroup = config.SectionGroups["UserSettingsGroup"];
        Hashtable sectionSettings = ConfigurationManager.GetSection(userSettingsGroup.Sections.Get("printerSettings").SectionInformation.SectionName) as Hashtable;
        return sectionSettings;
    }

    private static Hashtable GetConfigurationReportsFolderSection()
    {
        ExeConfigurationFileMap configMap = new ExeConfigurationFileMap();
        Configuration config = ConfigurationManager.OpenExeConfiguration(System.Reflection.Assembly.GetEntryAssembly().Location);
        ConfigurationSectionGroup userSettingsGroup = config.SectionGroups["UserSettingsGroup"];
        Hashtable sectionSettings = ConfigurationManager.GetSection(userSettingsGroup.Sections.Get("reportsFolderSettings").SectionInformation.SectionName) as Hashtable;
        return sectionSettings;
    }

    public static Hashtable GetStoredSettings(StoredSettingsType sectionType)
    {
        Hashtable table = new Hashtable();
        switch (sectionType)
        {
            case StoredSettingsType.DBConnection:
                {
                    ConfigurationSection section = GetConfigurationConnectionStringsSection();
                    ConnectionStringSettingsCollection col = ((ConnectionStringsSection)section).ConnectionStrings;

                    foreach (ConnectionStringSettings settings in col)
                    {
                        table.Add(settings.Name, settings.ConnectionString);
                    }

                    return table;
                }
            case StoredSettingsType.Printer:
                {
                    table = GetConfigurationPrinterSection();
                    return table;
                }

            case StoredSettingsType.ReportsFolder:
                {
                    table = GetConfigurationReportsFolderSection();
                    return table;
                }

        }
        return null;
    }
}

Is anything wrong with this code? Could it be more object-oriented?

Answer 1

• You don't use ExeConfigurationFileMap configMap in any of your methods just get rid of it. If you do copy&pasta you should always check if some cleaning needs to be done.

• You are calling this

  Configuration config = ConfigurationManager.OpenExeConfiguration(System.Reflection.Assembly.GetEntryAssembly().Location);
  ConfigurationSectionGroup userSettingsGroup = config.SectionGroups["UserSettingsGroup"];  
  

  in every GetXXX method. Extract it to a separate method.

• Something like

  table = GetConfigurationPrinterSection();
  return table;  
  

  doesn't buy you anything. Just return directly which makes it possible to reduce the scope of table to only the first case.

• The GetConfigurationConnectionStringsSection() method already returns a ConnectionStringsSection so there is no need to read it as ConfigurationSection only to cast it to a ConnectionStringsSection. Changing this would also make the ConnectionStringSettingsCollection col superfluous.

• To make your life a little bit easier you could use the var type at places where it is obvious from the right hand side of an assignment which type is meant.

• The lines where you read the e.g printerSettings are a little bit long.

If we would change the GetXXX methods like mentioned we would get

private static ConfigurationSectionGroup FetchConfigurationSectionGroup(string groupName)
{
    Configuration config = ConfigurationManager.OpenExeConfiguration(System.Reflection.Assembly.GetEntryAssembly().Location);
    return config.SectionGroups[groupName];
}  
private static string FetchSectionName(ConfigurationSectionGroup group, string sectionName)
{
    return group.Sections.Get(sectionName).SectionInformation.SectionName;
}  
private static Hashtable GetConfigurationPrinterSection()
{
    ConfigurationSectionGroup userSettingsGroup = FetchConfigurationSectionGroup("UserSettingsGroup");
    Hashtable sectionSettings = ConfigurationManager.GetSection(FetchSectionName(userSettingsGroup,"printerSettings")) as Hashtable;
    return sectionSettings;
}

private static Hashtable GetConfigurationReportsFolderSection()
{
    ConfigurationSectionGroup userSettingsGroup = FetchConfigurationSectionGroup("UserSettingsGroup");
    Hashtable sectionSettings = ConfigurationManager.GetSection(FetchSectionName(userSettingsGroup, "reportsFolderSettings")) as Hashtable;
    return sectionSettings;
}  

but as you see we still have some code duplication here, so a method FetchSetting(string groupName, string sectionName) would help us to remove that duplication like so

private static Hashtable FetchSetting(string groupName, string sectionName)
{
    ConfigurationSectionGroup userSettingsGroup = FetchConfigurationSectionGroup(groupName);
    return ConfigurationManager.GetSection(FetchSectionName(userSettingsGroup, sectionName)) as Hashtable;
}
private static ConfigurationSectionGroup FetchConfigurationSectionGroup(string groupName)
{
    Configuration config = ConfigurationManager.OpenExeConfiguration(System.Reflection.Assembly.GetEntryAssembly().Location);
    return config.SectionGroups[groupName];
}
private static string FetchSectionName(ConfigurationSectionGroup sectionGroup, string sectionName)
{
    return sectionGroup.Sections.Get(sectionName).SectionInformation.SectionName;
}
private static Hashtable GetConfigurationPrinterSection()
{
    return FetchSetting("UserSettingsGroup", "printerSettings");
}

private static Hashtable GetConfigurationReportsFolderSection()
{
    return FetchSetting("UserSettingsGroup", "reportsFolderSettings");
}  

which looks quite nice and tidy.

But wait, what about the ConnectionStringsSection ? If we remove everything which isn't necessary we will end with

private static ConnectionStringsSection GetConfigurationConnectionStringsSection()
{
    return (ConnectionStringsSection)FetchConfigurationSectionGroup("UserSettingsGroup")
        .SectionGroups.Get("StoredConnectionSettings")
        .Sections.Get("defaultConnectionString");
}  

but because the HashTable table will only be used for that value we should generate the HashTable there which will lead to

private static Hashtable GetConfigurationConnectionStringsSection()
{
    var section= (ConnectionStringsSection)FetchConfigurationSectionGroup("UserSettingsGroup")
        .SectionGroups.Get("StoredConnectionSettings")
        .Sections.Get("defaultConnectionString");

    Hashtable table = new Hashtable();
    ConnectionStringSettingsCollection col = section.ConnectionStrings;

    foreach (ConnectionStringSettings settings in section.ConnectionStrings)
    {
        table.Add(settings.Name, settings.ConnectionString);
    }

    return table;
}  

leaving the former GetStoredSettings() like this

public static Hashtable GetStoredSettings(StoredSettingsType sectionType)
{

    switch (sectionType)
    {
        case StoredSettingsType.DBConnection:
            {
                return GetConfigurationConnectionStringsSection();
            }

        case StoredSettingsType.Printer:
            {
                return GetConfigurationPrinterSection();
            }

        case StoredSettingsType.ReportsFolder:
            {
                return GetConfigurationReportsFolderSection(); ;
            }
        default:
            {
                return null;
            }
    }
}  

which looks strange with the given name, so let us rename the GetXXX methods which will lead to this

private static Hashtable FetchConnectionStringsSetting()
{
    var section= (ConnectionStringsSection)FetchConfigurationSectionGroup("UserSettingsGroup")
        .SectionGroups.Get("StoredConnectionSettings")
        .Sections.Get("defaultConnectionString");

    Hashtable table = new Hashtable();
    ConnectionStringSettingsCollection col = section.ConnectionStrings;

    foreach (ConnectionStringSettings settings in section.ConnectionStrings)
    {
        table.Add(settings.Name, settings.ConnectionString);
    }

    return table;
}
private static Hashtable FetchPrinterSetting()
{
    return FetchSetting("UserSettingsGroup", "printerSettings");
}

private static Hashtable FetchReportsFolderSetting()
{
    return FetchSetting("UserSettingsGroup", "reportsFolderSettings");
}
private static Hashtable FetchSetting(string groupName, string sectionName)
{
    ConfigurationSectionGroup userSettingsGroup = FetchConfigurationSectionGroup(groupName);
    return ConfigurationManager.GetSection(FetchSectionName(userSettingsGroup, sectionName)) as Hashtable;
}
private static ConfigurationSectionGroup FetchConfigurationSectionGroup(string groupName)
{
    Configuration config = ConfigurationManager.OpenExeConfiguration(System.Reflection.Assembly.GetEntryAssembly().Location);
    return config.SectionGroups[groupName];
}
private static string FetchSectionName(ConfigurationSectionGroup group, string sectionName)
{
    return group.Sections.Get(sectionName).SectionInformation.SectionName;
}
public static Hashtable GetStoredSettings(StoredSettingsType sectionType)
{

    switch (sectionType)
    {
        case StoredSettingsType.DBConnection:
            {
                return FetchConnectionStringsSetting();
            }

        case StoredSettingsType.Printer:
            {
                return FetchPrinterSetting();
            }

        case StoredSettingsType.ReportsFolder:
            {
                return FetchReportsFolderSetting(); ;
            }
        default:
            {
                return null;
            }
    }
}