Skip to main content
Code Review

Return to Answer

replaced http://stackoverflow.com/ with https://stackoverflow.com/
Source Link

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

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

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

Source Link
mjolka
  • 16.3k
  • 2
  • 30
  • 73

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;
 }
}
lang-cs

AltStyle によって変換されたページ (->オリジナル) /