Find all collection properties of an object and get their values

Question

I have a chunk of code that get's all of the ICollection properties of the passed in object and then takes all of their values and adds them to another ICollection.

End goal is to see which properties have child objects in them and thus finding out how many dependents the object has.

The object is an EntityFramework entity so the children would be other entities. I'm not entirely convinced that I'm actually grabbing the correct properties (although in my tests so far it works) and I'm not convinced I'm parsing them efficiently either. Here's the code, and I welcome any recommendations:

public class DependencyValidator<TEntity> :
 IDependencyValidator<TEntity>
 where TEntity : class {
 private readonly Type IEnumerableType = typeof(IEnumerable);
 private readonly Type StringType = typeof(String);
 public IEnumerable<KeyValuePair<string, IEnumerable<string>>> Dependencies(
 TEntity entity) {
 if (entity == null) {
 return Enumerable.Empty<KeyValuePair<string, IEnumerable<string>>>();
 }
 ICollection<KeyValuePair<string, IEnumerable<string>>> dependents = new List<KeyValuePair<string, IEnumerable<string>>>();
 entity.GetType().GetProperties().Where(
 p =>
 this.IEnumerableType.IsAssignableFrom(p.PropertyType)
 && !this.StringType.IsAssignableFrom(p.PropertyType)).ForEach(
 p => {
 IEnumerable values = (p.GetValue(entity) as IEnumerable);
 ICollection<string> children = new List<string>();
 foreach (var value in values) {
 children.Add(value.ToString());
 }
 dependents.Add(new KeyValuePair<string, IEnumerable<string>>(p.Name, children));
 });
 return dependents;
 }
}

Answer 1

First up, let's clean up the formatting a bit.

public class DependencyValidator<TEntity> :
 IDependencyValidator<TEntity>
 where TEntity : class
{
 private readonly Type IEnumerableType = typeof(IEnumerable);
 private readonly Type StringType = typeof(String);
 public IEnumerable<KeyValuePair<string, IEnumerable<string>>> Dependencies(TEntity entity)
 {
 if (entity == null)
 {
 return Enumerable.Empty<KeyValuePair<string, IEnumerable<string>>>();
 }
 ICollection<KeyValuePair<string, IEnumerable<string>>> dependents = new List<KeyValuePair<string, IEnumerable<string>>>();
 entity.GetType().GetProperties().Where(
 p =>
 this.IEnumerableType.IsAssignableFrom(p.PropertyType)
 && !this.StringType.IsAssignableFrom(p.PropertyType)).ForEach(
 p =>
 {
 IEnumerable values = (p.GetValue(entity) as IEnumerable);
 ICollection<string> children = new List<string>();
 foreach (var value in values)
 {
 children.Add(value.ToString());
 }
 dependents.Add(new KeyValuePair<string, IEnumerable<string>>(p.Name, children));
 });
 return dependents;
 }
}

Now let's use the var keyword to make it less verbose.

public class DependencyValidator<TEntity> :
 IDependencyValidator<TEntity>
 where TEntity : class
{
 private readonly Type IEnumerableType = typeof(IEnumerable);
 private readonly Type StringType = typeof(String);
 public IEnumerable<KeyValuePair<string, IEnumerable<string>>> Dependencies(TEntity entity)
 {
 if (entity == null)
 {
 return Enumerable.Empty<KeyValuePair<string, IEnumerable<string>>>();
 }
 var dependents = new List<KeyValuePair<string, IEnumerable<string>>>();
 entity.GetType().GetProperties().Where(
 p =>
 this.IEnumerableType.IsAssignableFrom(p.PropertyType)
 && !this.StringType.IsAssignableFrom(p.PropertyType)).ForEach(
 p =>
 {
 var values = (p.GetValue(entity) as IEnumerable);
 var children = new List<string>();
 foreach (var value in values)
 {
 children.Add(value.ToString());
 }
 dependents.Add(new KeyValuePair<string, IEnumerable<string>>(p.Name, children));
 });
 return dependents;
 }
}

Your ForEach method seems to be non-standard, so I'd like to get rid of that.

public class DependencyValidator<TEntity> :
 IDependencyValidator<TEntity>
 where TEntity : class
{
 private readonly Type IEnumerableType = typeof(IEnumerable);
 private readonly Type StringType = typeof(String);
 public IEnumerable<KeyValuePair<string, IEnumerable<string>>> Dependencies(TEntity entity)
 {
 if (entity == null)
 {
 return Enumerable.Empty<KeyValuePair<string, IEnumerable<string>>>();
 }
 var dependents = new List<KeyValuePair<string, IEnumerable<string>>>();
 var properties = entity.GetType()
 .GetProperties()
 .Where(p => this.IEnumerableType.IsAssignableFrom(p.PropertyType) && !this.StringType.IsAssignableFrom(p.PropertyType));
 foreach (var property in properties)
 {
 var values = property.GetValue(entity) as IEnumerable;
 var children = new List<string>();
 foreach (var value in values)
 {
 children.Add(value.ToString());
 }
 dependents.Add(new KeyValuePair<string, IEnumerable<string>>(property.Name, children));
 }
 return dependents;
 }
}

ReSharper suggests replacing the inner loop with a LINQ expression, which I think looks alright.

public class DependencyValidator<TEntity> :
 IDependencyValidator<TEntity>
 where TEntity : class
{
 private readonly Type IEnumerableType = typeof(IEnumerable);
 private readonly Type StringType = typeof(String);
 public IEnumerable<KeyValuePair<string, IEnumerable<string>>> Dependencies(TEntity entity)
 {
 if (entity == null)
 {
 return Enumerable.Empty<KeyValuePair<string, IEnumerable<string>>>();
 }
 var dependents = new List<KeyValuePair<string, IEnumerable<string>>>();
 var properties = entity.GetType()
 .GetProperties()
 .Where(p => this.IEnumerableType.IsAssignableFrom(p.PropertyType) && !this.StringType.IsAssignableFrom(p.PropertyType));
 foreach (var property in properties)
 {
 var values = property.GetValue(entity) as IEnumerable;
 var children = (from object value in values select value.ToString()).ToList();
 dependents.Add(new KeyValuePair<string, IEnumerable<string>>(property.Name, children));
 }
 return dependents;
 }
}

Finally, we can make the fields static. ReSharper warns about this, but I think it's fine in this case.

 public class DependencyValidator<TEntity> :
 IDependencyValidator<TEntity>
 where TEntity : class
{
 private static readonly Type IEnumerableType = typeof(IEnumerable);
 private static readonly Type StringType = typeof(string);
 public IEnumerable<KeyValuePair<string, IEnumerable<string>>> Dependencies(TEntity entity)
 {
 if (entity == null)
 {
 return Enumerable.Empty<KeyValuePair<string, IEnumerable<string>>>();
 }
 var dependents = new List<KeyValuePair<string, IEnumerable<string>>>();
 var properties = entity.GetType()
 .GetProperties()
 .Where(p => IEnumerableType.IsAssignableFrom(p.PropertyType) && !StringType.IsAssignableFrom(p.PropertyType));
 foreach (var property in properties)
 {
 var values = property.GetValue(entity) as IEnumerable;
 var children = (from object value in values select value.ToString()).ToList();
 dependents.Add(new KeyValuePair<string, IEnumerable<string>>(property.Name, children));
 }
 return dependents;
 }
}

• svick: I would get rid of the fields altogether.

Answer 2

Issues

Possible null reference exceptions:

both values as any value could be null.

foreach (var value in values) {
 children.Add(value.ToString());
}

Improve robustness of finding properties:

make sure to filter out properties with a public getter that aren't indexers.

entity.GetType().GetProperties()

entity.GetType().GetProperties().Where(p => p.CanRead && p.GetGetMethod() != null 
 && p.GetIndexParameters().Length == 0)

---

Refactored solution

Original solution provided by mjolka.

• Issues are fixed
• Generator pattern (yield) instead of verbose return statements
• A couple of small syntax changes using newer C# features

code

 public IEnumerable<KeyValuePair<string, IEnumerable<string>>> Dependencies(TEntity entity) 
 {
 if (entity == null)
 {
 yield break;
 }
 var properties = entity.GetType()
 .GetProperties()
 .Where(p => typeof(IEnumerable).IsAssignableFrom(p.PropertyType) 
 && !typeof(string).IsAssignableFrom(p.PropertyType)
 && p.CanRead && p.GetGetMethod() != null 
 && p.GetIndexParameters().Length == 0);
 foreach (var property in properties)
 {
 if (property.GetValue(entity) is IEnumerable values)
 {
 var children = (
 from object value 
 in values
 where value != null
 select value.ToString()
 ).ToList();
 yield return new KeyValuePair<string, IEnumerable<string>>
 (property.Name, children);
 }
 }
 }