I want to store some user settings into a file, so that the users get the same experience on all machines they're working on. The built-in user.config
turned out to be insufficient, as I can't store Nullable<T>
values in it.
I created a simple class for my program that stores the user settings in an XML file. This is how the file may look like:
<?xml version="1.0" encoding="utf-16"?>
<UserSettings>
<Section1>
<SubSectionA>
<ASetting1>value</ASetting1>
<ASetting2>value</ASetting2>
<ASetting3>value</ASetting3>
</SubSectionA>
<SubSectionB>
<BSetting1>value</BSetting1>
</SubSectionB>
<SubSectionC>
<CSetting1>value</CSetting1>
<CSetting2>value</CSetting2>
</SubSectionC>
</Section1>
<Section2>
<SubSectionA>
<ASetting1>value</ASetting1>
<ASetting2>value</ASetting2>
<ASetting3>value</ASetting3>
</SubSectionA>
<SubSectionB>
<BSetting1>value</BSetting1>
</SubSectionB>
<SubSectionC>
<CSetting1>value</CSetting1>
<CSetting2>value</CSetting2>
</SubSectionC>
</Section2>
<Section3>
...
</Section3>
</UserSettings>
Finally, this is my UserSettings
class:
public class UserSettings
{
static UserSettings instance;
private static UserSettings Instance
{
get
{
if (instance == null)
instance = new UserSettings();
return instance;
}
}
// This is a sample setting
public static DateTime? Section1_SubSectionA_ASetting1
{
get
{
DateTime storedValue;
if (DateTime.TryParse(Instance.GetSetting("Section1", "SubSectionA", "ASetting1"), out storedValue))
return storedValue;
else
// Return the default application value for this setting
return null;
}
set { Instance.SaveSetting(value, "Section1", "SubSectionA", "ASetting1"); }
}
// Static
// ---------------------------------------------------------------------
// Instance
readonly string settingsFilePath;
XDocument settingsFile;
private UserSettings()
{
this.settingsFilePath = Environment.GetFolderPath(Environment.SpecialFolder.ApplicationData) + @"\MyCompany\MyApplication\UserSettings.xml";
// Create the settings file if neccessary
if (!File.Exists(this.settingsFilePath))
{
XDocument newSettingsFile = new XDocument(
new XDeclaration("1.0", "utf-16", "true"),
new XElement("UserSettings"));
// Create the folders if they not exist
new FileInfo(this.settingsFilePath).Directory.Create();
newSettingsFile.Save(this.settingsFilePath);
this.settingsFile = newSettingsFile;
}
else
this.settingsFile = XDocument.Load(this.settingsFilePath);
}
private string GetSetting(params string[] elementNames)
{
XElement element = this.settingsFile.Root;
// Walk through the given element names
foreach (string elementName in elementNames)
{
XElement child = element.Elements(elementName).FirstOrDefault();
// If an element does not exist, the setting has not been set yet, return the default value
if (child == null)
return default(string);
element = child;
}
return element.Value;
}
private void SaveSetting(object value, params string[] elementNames)
{
XElement element = this.settingsFile.Root;
foreach (string elementName in elementNames)
{
XElement child = element.Elements(elementName).FirstOrDefault();
// If an element does not exist, the setting has not been set yet, create the element and move on
if (child == null)
{
child = new XElement(elementName);
element.Add(child);
}
element = child;
}
element.Value = value.ToString();
// Save the file to disk
this.settingsFile.Save(this.settingsFilePath);
}
}
Is there any way I can improve my code? You can assume that there are no sibling XML elements that share the same name. If so, the code should always read from and write to the first one, ignoring the others.
1 Answer 1
I see a few things that you could improve. I also consider it as an exercise for reading xml files so I won't suggest you to use a xml serializer to directly turn everything into an object.
- You shouldn't call other methods as parameters not only because the call is extremely long but it's also easier to debug it later if you can see the result of that call:
so instead of
if (DateTime.TryParse(Instance.GetSetting("Section1", "SubSectionA", "ASetting1"), out storedValue))
this would be better
var setting = Instance.GetSetting("Section1", "SubSectionA", "ASetting1");
if (DateTime.TryParse(setting, out storedValue))
- The
GetSetting
method you can make shorter by usingXPath
It's just an example, you should build the path dynamicly based on params string[] elementNames
var xPath = "//UserSettings/Section1/SubSectionA/ASetting1";
var xSetting = xConfig.XPathSelectElement(xPath);
Both methods
GetSetting
andSaveSetting
use almost the same code, it looks like Copy&Paste. You should create a single method for getting a setting element to get rid of the repetitions likeXElement GetElement(params string[] elementNames)
This is not a good name for a method
Section1_SubSectionA_ASetting1
, give it a more meaningful name likeGetBirthdate
SmartConfig
;-] (link in my profile) \$\endgroup\$