Handling of Null Values in Templating object

[ghost]

Two Options

1. Better Error Message

document = data.GetType().GetProperties().Aggregate(document, (current, property) =>
{
    if (property.GetValue(data, null) == null)
    {
        throw new NullReferenceException("Field is null : " + property.Name);
    }
    return ReplaceTemplateField(current, property.Name,  property.GetValue(data, null).ToString());
});

• Less Forgiving
• More Feedback

  2. Swallow the nulls

 document = data.GetType().GetProperties().Aggregate(document,
        (current, property) => ReplaceTemplateField(current, property.Name, 
        String.Format("{0}",property.GetValue(data, null))));

• No Exception When maybe there should be
• Approximate 15% performance hit (String.Format vs .ToString())

  Thoughts?

Feels like this kind of decision is best left to the person in charge of the code base. Which way feels more true to the vision?

As it currently stands it simply gives an error message that says "Object reference not set to an instance of an object."

[becdetat]

My feel is that null should translate to an empty string, I've encountered lots of cases where null is used interchangeably with string.Empty. I'm happy with a performance hit but it could be implemented like:

=> {
    var value = property.GetValue(data, null);
    return value == null ? string.Empty : value.ToString();
}

[ghost]

It should translate to an empty string. Changing up the ReplaceTemplateField to accept an object instead of a string and then doing the null check in there also fixes for fields and ExpandoObjects and whatever else to come

   public static string ReplaceTemplateField(string document, string fieldName, object fieldValue)
    {
        return document.Replace(TOKEN_START + fieldName + TOKEN_END, fieldValue == null ? string.Empty : fieldValue.ToString());
    }

It also cleans up the ParseTemplate Method.

Pull Request incoming