How to prevent adding XML element if XElement is Null?

Question

I have a simple XML document that looks like this:

<Person>
  <LastName>LastName1</LastName>
  <FirstName>FirstName1</FirstName>
  <MiddleName>MiddleName1</MiddleName>
  <Suffix>Suffix1</Suffix>
</Person>

However I have a constraint where I am not allowed to add empty tags. Therefore if the Suffix value does not exist, I cannot use <Suffix /> or validation will fail.

I'm composing the XML structure using XElement objects from different classes that return their respective XML via a returned XElement object from a .ToXML() method. I need to check per element to see if the XElement being returned is null. If that's the case it has to be like that line never existed. I'm trying to use the ?? operator but I'm getting the error that ?? left operand is never null. I had the code as follows:

public XElement ToXML()
{
  return new XElement("Employee",
    new XElement(this.LastName.ToXML()) ?? null,
    new XElement(this.FirstName.ToXML()) ?? null,
    new XElement(this.MiddleName.ToXML()) ?? null,
    new XElement(this.Suffix.ToXML()) ?? null);
}

How can I check per XML node to see if the XElement object being returned is null and if so ignore adding/composing that node all together? Any help is appreciated, thanks!

What is the return type of `ToXML()` method? If it is `XElement`, why are you creating a new `XElement` around it? — Mohammad Dehghan, Feb 04 '13 at 16:36
No you are correct and I should not have used `new` when the method already returns a `XElement` instance. I should have not put that in the sample, but I will leave it for future readers. — atconway, Feb 04 '13 at 16:40

score 3 · Answer 1 · answered Feb 04 '13 at 16:30

You should be using this code instead:

public XElement ToXML()
{
    var children = new[]
    {
        this.LastName.ToXML(),
        this.FirstName.ToXML(),
        this.MiddleName.ToXML(),
        this.Suffix.ToXML()
    };

    return new XElement("Employee", children.Where(x => x != null));
}

Please note that your code has several issues:

The null-coalescing operator (??) is an operator that returns the right hand value when the left hand value is null. Making the right hand value null is completely useless.
Mixing a new statement with the ?? operator is completely useless, too, because the result of new will never be null.
The new XElement part also seems pretty useless. I assume that ToXML already returns an XElement, so why create a new instance?

Correct on #3, it was indeed useless because the method already returns the element. — atconway, Feb 04 '13 at 16:37

user7116 · Accepted Answer · 2013-02-04T16:43:42.167

2

A constructor in C# will either return a non-null reference to the object or will throw an exception. It will not return null*.

As for your problem, why not:

return new XElement("Employee",
    this.LastName.ToXML(),
    this.FirstName.ToXML(),
    this.MiddleName.ToXML(),
    this.Suffix.ToXML());

And just have each of those ToXML methods return null if none exist?

Or if your case is the properties themselves are null:

return new XElement("Employee",
    this.LastName != null ? this.LastName.ToXML() : null, /* null is ignored */
    this.FirstName != null ? this.FirstName.ToXML() : null,
    this.MiddleName != null ? this.MiddleName.ToXML() : null,
    this.Suffix != null ? this.Suffix.ToXML() : null);

I also just realized perhaps you always get back an XElement, but it may be empty, in that case:

var elements = new[] { this.LastName.ToXML(), this.FirstName.ToXML(), ...

// use IsEmpty to filter out those that are <Element/>
return new XElement("Employee",
    elements.Where(ee => ee != null && !ee.IsEmpty));

_{*I believe there is an interesting edge case where you could get this from a COM interface instantiation, but we'll ignore all "strange" coding.}

edited Feb 04 '13 at 16:43

answered Feb 04 '13 at 16:30

user7116

63,008
17
141
172

If I went with your 1st method and it returns `null`, then you say it will automatically not be added a node to the `XElement` root correct? Mening I don't have to explicitly check as I thought and in your 2nd sample? – atconway Feb 04 '13 at 16:42
1

@atconway: that is correct, if they return `null` that is fine. Obviously it can't be added as a node as there is nothing it could know about the node, attribute, etc to add. – user7116 Feb 04 '13 at 16:45
If the ordering of the elements in `elements` shown in your 3rd sample are *preserved* then I think that seems to be a decent solution too. – atconway Feb 04 '13 at 16:45
@atconway: Yes they are preserved. – user7116 Feb 04 '13 at 16:46
So the only reason to use #3 is when my methods *always* return an instance of `XElement` (even if empty) as opposed to returning `null`? If `null` *can* be returned than #1 is probably best, correct? Want to make sure I understand *when* I need to explicitly check for `null` vs. not having to. – atconway Feb 04 '13 at 16:48
1

@atconway: Yes the 3rd method is useful if they always return it. I included the null check to "future" proof against an exception being thrown in the `Where` clause. – user7116 Feb 04 '13 at 16:50
very nice, thank you sir. – atconway Feb 04 '13 at 16:54

How to prevent adding XML element if XElement is Null?

2 Answers2