1
\$\begingroup\$

I am parsing an XML file and trying to get the guid of a node where it has an attribute "name" and a value of "Name" like so:

<property name="Name" guid="7eca00b3-38e0-f0e8-9b72-2b822b21de6b" />

string propertyGuid = propertyList.Where(x => x.name == "Name").FirstOrDefault()?.ToString();
if (propertyGuid == (object)null)     
     throw new ArgumentNullException();

So I just found out that there are nodes that don't have a "Name" value but have a "Code" value. Which lets me check for another null value again

<property name="Code" guid="8bca00b3-38e0-f0e8-9b72-2b822b21de7a" />

string propertyGuid = propertyList.Where(x => x.name == "Name").FirstOrDefault()?.ToString();
if (propertyGuid == (object)null)
{
     propertyGuid = propertyList.Where(x => x.name == "Code").FirstOrDefault()?.ToString();
     if (propertyGuid == (object)null)
          throw new ArgumentNullException();
}

How do I refactor my code such that when another criteria (e.g "ID") is introduced in the future, I don't have to make another nested null check?

EDIT:

The check should be the other way around.

if ((object)propertyGuid == null)
\$\endgroup\$
4
  • 2
    \$\begingroup\$ null is null, no need to cast to an object \$\endgroup\$ – 404 Nov 29 '17 at 11:27
  • \$\begingroup\$ Is x your custom type with a name property because to me it looks a lot like pseudocode. \$\endgroup\$ – t3chb0t Nov 29 '17 at 17:57
  • \$\begingroup\$ @t3chb0t yes, it is generated from an XSD model \$\endgroup\$ – jmc Nov 30 '17 at 7:06
  • \$\begingroup\$ @eurotrash edited the code, it should be the other way around if ((object)propertyGuid == null) \$\endgroup\$ – jmc Nov 30 '17 at 7:40
4
\$\begingroup\$

You can just go with :

// Just check both case at the same time
string propertyGuid = propertyList.Where(x => x.name == "Name" || x.name == "Code").FirstOrDefault()?.ToString();
if (string.IsNullOrEmpty(propertyGuid)) // Same check as you did but written differently
{         
    throw new ArgumentNullException();
}

Then for the future, I guess that you can have Guids on node you don't want to extract. You could have properties on a list or array :

List<string> possibleName = new List<string>() {"Code", "Name"};

And then calling an extract function :

Public static string GetGuid(List<string> possibleName, Whatever propertyList)
{
    string propertyGuid = string.Empty;
    foreach (string name on possibleName)
    {
        propertyGuid = propertyList.Where(x => x.name == name).FirstOrDefault()?.ToString();
        if (!string.IsNullOrEmpty(propertyGuid))     
        {
             return (propertyGuid);
        }
    }
    throw new ArgumentNullException();
}

Ps : I'd like to add that if propertyList is a NodeList you won't get what you want.

\$\endgroup\$
0
\$\begingroup\$

You can reduce some code and [IMO] increase readability with the following:

validPropertyValues should come from whatever source you want to manage this - database, config file, constant in code.

//valid values in the name property
var validPropertyValues = new List<string> { "Code", "Name" };

//check to see if name contains any of the accepted values
if (propertyList.Any(d => validPropertyValues.Contains(d.name)))
{
    //returns guid from the list. 
    //Using First() will throw an exception if guid doesn't exist

    //option 1
    //If guid will be populated if it exists
    return propertyList.Select(d => d.guid).First();

    //option 2
    //this version is only needed if guid can exist AND be empty
    var guidValue = propertyList.Select(d => d.guid).First();
    if (string.IsNullOrWhiteSpace(guidValue))
    {
        throw new ArgumentNullException();
    }

    return guidValue;
}
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.