My name
is
Jon Skeet

How to parse an xml file and return default value if no element found

I wrote a simple method in C# to parse a given xml file and return the value of a specific node. It works fine but I'd also like to return a default value if the node is not found instead of throwing an exception. How can I do this? Can the method be written better? Thanks for any help offered. John

public static string ReadConfigurationFile(string configurationFileName, string root, string section, string name)
{
    try
    {
        String currentDirectory = Path.GetDirectoryName(System.Reflection.Assembly.GetExecutingAssembly().Location);
        configFilePath = Directory.GetParent(currentDirectory) + configurationFolder + configurationFileName;

        XDocument configXML = XDocument.Load(configFilePath);
        var result = from setting in
                         configXML.Descendants(root)
                            .Descendants(section)
                     select setting.Element(name).Attribute("value").Value;
        return result.First();
    }
    catch (Exception ex)
    {
        return string.Empty;
    }
}

Here's a sample of the XML file I parse:

<?xml version='1.0' encoding='utf-8'?>
<automationSettings>
    <vmDomain>
        <domainName value = "Domain"/>
        <domainUsername value = "username"/>
        <domainPassword value = "password"/>
    </vmDomain>
</automationSettings>

Let's start by getting rid of the exception "handling". The node not being found is a "reasonable to expect" error, and one we're going to ensure doesn't result in an exception. Other exceptions - such as the file not being found at all, or not being valid XML - should probably be thrown.

Next, let's stop using a query expression - when you're only using a select clause, it doesn't really buy you anything.

As a next step, I'm going to stop assigning to configFilePath which is presumably a field. Writing to that field as a side-effect seems like a really bad idea to me. Let's use Path.Combine to combine bits of a path as well...

So now we've got:

// Work in progress!
public static string ReadConfigurationFile(
    string configurationFileName,
    string root,
    string section,
    string name)
{
    string currentDirectory = Path.GetDirectoryName(
        Assembly.GetExecutingAssembly().Location);
    var fullConfigPath = Path.Combine(
        Directory.GetParent(currentDirectory),
        configurationFolder,
        configurationFileName);

    var configXml = XDocument.Load(fullConfigPath);
    return configXml.Descendants(root)
                    .Descendants(section)
                    .Select(x => x.Element(name).Attribute("value").Value
                    .First();
}

That's now going to throw an exception if it can't find either the element or the attribute. We can fix that like this:

return configXml.Descendants(root)
                .Descendants(section)
                .Elements(name)
                .Select(x => (string) x.Attribute("value"))
                .FirstOrDefault();

Now, if Elements() returns an empty sequence, there'll be nothing to select and FirstOrDefault() will return null. If there is an element and it doesn't have the value attribute, x.Attribute("value") will return null, and the explicit conversion from XAttribute to string will return null.

While we're at it, we're only using configXml for one call, so let's inline that, leaving us with:

public static string ReadConfigurationFile(
    string configurationFileName,
    string root,
    string section,
    string name)
{
    string currentDirectory = Path.GetDirectoryName(
        Assembly.GetExecutingAssembly().Location);
    var fullConfigPath = Path.Combine(
        Directory.GetParent(currentDirectory),
        configurationFolder,
        configurationFileName);

    return XDocument.Load(fullConfigPath)
                    .Descendants(root)
                    .Descendants(section)
                    .Elements(name)
                    .Select(x => (string) x.Attribute("value"))
                    .FirstOrDefault();
}

Now, that returns null rather than the empty string that your original code does. I'd argue that's better, because:

It allows the caller to differentiate "the setting was provided but empty" from "the setting wasn't provided"
It allows the caller to use the null-coalescing operator to specify a default value:
```
var setting = ReadConfigurationFile("a", "b", "c", "d") ?? "some default value";
```

See more on this question at Stackoverflow