patterncsharpMinor
Simple object comparer
Viewed 0 times
objectsimplecomparer
Problem
Creating a simple object compare function for public system types in a class meaning only looking at the first level of a class no breaking into an object and comparing sub properties.
It performs very quickly however I am unsure about the extensibility of it for the future should criteria evolve
Note: Reads
It performs very quickly however I am unsure about the extensibility of it for the future should criteria evolve
public static IEnumerable Compare(this T1 Object1, T2 Object2) where T1 : IComparer
{
if (Object2 == null)
throw new NullReferenceException("Parameter object not instantiated");
Type TypeOfObject1 = typeof(T1);
PropertyInfo[] Object1Properties = TypeOfObject1.GetProperties(BindingFlags.Public | BindingFlags.Instance);
Type TypeOfObject2 = typeof(T2);
PropertyInfo[] Object2Properties = TypeOfObject2.GetProperties(BindingFlags.Public | BindingFlags.Instance);
if (Object1Properties.Count() == 0 || Object2Properties.Count() == 0)
throw new NullReferenceException("No public properties found");
var DifferencesFound = Object1Properties
.Join(Object2Properties,
Object1Property => Object1Property.Name,
Object2Property => Object2Property.Name,
(Object1Property, Object2Property) => new { Object1Property = Object1Property, Object2Property = Object2Property })
.Where(x => ((Type.GetTypeCode(x.Object1Property.PropertyType) != TypeCode.Object
&& Type.GetTypeCode(x.Object2Property.PropertyType) != TypeCode.Object
)
&& !x.Object1Property.GetValue(Object1).Equals(x.Object2Property.GetValue(Object2)))
)
.Select(x => x.Object1Property.GetCustomAttributes(typeof(DisplayNameAttribute), false).SingleOrDefault() as string ?? x.Object1Property.Name)
.AsEnumerable();
return DifferencesFound;
}Note: Reads
DisplayName attribute if it is not found then uses nameSolution
First of all the name,
Maybe you have a reason for that, in code you're not showing, but there is no requirement for
Names reloaded. Usually local variables (and parameters) are camelCase.
Both
This code:
Can be simplified to:
However if caller specifies the generic types like this (just an example...):
You may get the wrong result (what do you expect here? To get properties of
You call
You may want to filter properties to feed
Only when
Is there a reason you do not compare reference types? Checking
You might want to move comparison to a separate method where you check for
Name selection can be simplified:
Why don't you move it into a separate method
Here you do not need
Compare doesn't explain what this method does (and I had to read all the code to have an idea). Don't be afraid of longer names like GetMatchedPropertiesWithDifferentValues() (or something like that...).Maybe you have a reason for that, in code you're not showing, but there is no requirement for
T1 to implement IComparer (I'd guess IComparable but...IComparer?)Names reloaded. Usually local variables (and parameters) are camelCase.
Both
Object1 and Object2 can be null, check for both. You should never ever throw NullReferenceException, if a function argument is null and it should not then you have to throw ArgumentNullException.This code:
Type TypeOfObject1 = typeof(T1);
PropertyInfo[] Object1Properties = TypeOfObject1.GetProperties(BindingFlags.Public | BindingFlags.Instance);Can be simplified to:
var object1Properties = typeof(T1).GetProperties();However if caller specifies the generic types like this (just an example...):
Compare(new Derived(), new Derived());You may get the wrong result (what do you expect here? To get properties of
Base or of Derived?) Just be aware of this (and document it).You call
Count() on PropertyInfo[]. LINQ implementation is smart enough but it really has no sense to avoid using Length property. Again do not throw NullReferenceException. Is there any null object? No, just an object without public properties then you can throw ArgumentException (possibly specialized for the two parameters).You may want to filter properties to feed
Join() (what you're doing later in Where() clause) before the Join(). Also, move it to a separate GetEligiblePropertiesForComparison() method.Only when
T1 and T2 coincide (maybe a separate overload?): if you just get the intersection of the two lists you can skip creating the temporary objects.Is there a reason you do not compare reference types? Checking
TypeCode works pretty well but moving this check to a separate method is even better. Also consider x.PropertyType.IsPrimitiveType || x.PropertyType == typeof(string) (if it's what you meant, comparison of DBNull can be tricky).You might want to move comparison to a separate method where you check for
IEquatable or IComparable or IComparable implementation (defaulting to EqualityComparer if types coincide).Name selection can be simplified:
x.Object1Property.GetCustomAttribute()?.DisplayName
?? x.Object1Property.Name;Why don't you move it into a separate method
string GetPropertyDisplayName(PropertyInfo)?Here you do not need
AsEnumerable().Code Snippets
Type TypeOfObject1 = typeof(T1);
PropertyInfo[] Object1Properties = TypeOfObject1.GetProperties(BindingFlags.Public | BindingFlags.Instance);var object1Properties = typeof(T1).GetProperties();Compare<Base, Base>(new Derived(), new Derived());x.Object1Property.GetCustomAttribute<DisplayNameAttribute>()?.DisplayName
?? x.Object1Property.Name;Context
StackExchange Code Review Q#161842, answer score: 2
Revisions (0)
No revisions yet.