HiveBrain v1.2.0
Get Started
← Back to all entries
patterncsharpMinor

Application Settings Helper

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
helpersettingsapplication

Problem

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);
ConfigurationSec

Solution

-
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:
{
retu

Code Snippets

Configuration config = ConfigurationManager.OpenExeConfiguration(System.Reflection.Assembly.GetEntryAssembly().Location);
ConfigurationSectionGroup userSettingsGroup = config.SectionGroups["UserSettingsGroup"];
table = GetConfigurationPrinterSection();
return table;
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;
}
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");
}
private static ConnectionStringsSection GetConfigurationConnectionStringsSection()
{
    return (ConnectionStringsSection)FetchConfigurationSectionGroup("UserSettingsGroup")
        .SectionGroups.Get("StoredConnectionSettings")
        .Sections.Get("defaultConnectionString");
}

Context

StackExchange Code Review Q#122531, answer score: 2

Revisions (0)

No revisions yet.