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

Storing user settings using LINQ to XML

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

Problem

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





value
value
value


value


value
value




value
value
value


value


value
value



...




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.settingsFileP

Solution

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 using XPath



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 and SaveSetting 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 like XElement GetElement(params string[] elementNames)

-
This is not a good name for a method Section1_SubSectionA_ASetting1, give it a more meaningful name like GetBirthdate

Code Snippets

if (DateTime.TryParse(Instance.GetSetting("Section1", "SubSectionA", "ASetting1"), out storedValue))
var setting = Instance.GetSetting("Section1", "SubSectionA", "ASetting1");
if (DateTime.TryParse(setting, out storedValue))
var xPath = "//UserSettings/Section1/SubSectionA/ASetting1";
var xSetting = xConfig.XPathSelectElement(xPath);

Context

StackExchange Code Review Q#102163, answer score: 2

Revisions (0)

No revisions yet.