What is wrong with this object being capable of telling how many of its properties are not null/whitespace/empty?

I am trying to figure out why the following code throws a StackOverflowException (I am finally posting a StackoverflowException in SO!).

Debugging seems to point out that the p.GetValue(this) is generating further calls.

What is actually triggering the infinite call chain ? Is it because p.GetValue(this) ultimately returns an instance of the current object, and thus acts as if constructing a new instance (and every object that constructs itself within its construction will lead to Stackoverflow exceptions) ?

My intent with the following code is to have an object being capable of telling how many of its properties have null/space/empty values.

public class A
{
    public string S1 { get; set; }
    public string S2 { get; set; }

    public int NonInitializedFields
    {
        get
        {
            int nonNullFields = 0;

            var properties = this.GetType().GetProperties();
            foreach (var p in properties)
            {
                var value = p.GetValue(this);
                if (value == null || string.IsNullOrWhiteSpace(value.ToString()))
                    nonNullFields++;
            }

            return nonNullFields;
        }
    }
}

//the following throws a StackOverflowException (the construction line itself)
A a1 = new A1{ S1 = "aaa"};
Console.WriteLine(a1.NonInitializedFields);

P.S. My idea originally involves only simple string properties, nothing else, so whatever problems may arise with this approach with other types are not relevant.

Jon Skeet
people
quotationmark

You have a property, which, when you execute the "get" accessor, finds all the properties and fetches their value. So it executes itself, recursively.

If you only want string properties, you should check the property type before fetching the value:

var properties = GetType().GetProperties().Where(p => p.PropertyType == typeof(string));

At that point, as your NonInitializedFields property doesn't have a return type of string, it won't be executed.

Personally I would make this a method call rather than a property anyway, mind you. That would also fix the issue, as the method wouldn't find itself when looking for properties.

I would also rename it, as:

  • A property isn't necessarily backed by a field
  • A field could be explicitly initialized as null or a reference to a string containing only whitespace

A method called GetNonWhitespaceStringPropertyCount() would be more accurate, IMO. You can also make the whole implementation a LINQ query:

return GetType().GetProperties()
                .Where(p => p.PropertyType == typeof(string))
                .Select(p => p.GetValue(this))
                .Count(v => !string.IsNullOrWhitespace((string) v));

Note that I've fixed the next issue in your code - you're meant to be counting non-null/empty values, but you're actually counting null/empty ones.

people

See more on this question at Stackoverflow