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?

• Phrancis: Welcome to Code Review. I hope you get some great answers!

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;
 }
 }
}