4
\$\begingroup\$

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

public static IEnumerable<string> Compare<T1, T2>(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 name

\$\endgroup\$

2 Answers 2

2
\$\begingroup\$

First of all the name, 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<Base, Base>(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<T> or IComparable<T> or IComparable implementation (defaulting to EqualityComparer<T> if types coincide).

Name selection can be simplified:

x.Object1Property.GetCustomAttribute<DisplayNameAttribute>()?.DisplayName
    ?? x.Object1Property.Name;

Why don't you move it into a separate method string GetPropertyDisplayName(PropertyInfo)?

Here you do not need AsEnumerable().

\$\endgroup\$
1
  • \$\begingroup\$ Very thorough and your assumptions are largely correct. IComparer is just an interface to restrict usage of the method and to add any additional functions that may intertwine in the future with the compare function, though I will rename it to be more adept. Have some refactoring to do now thank you . \$\endgroup\$
    – SCFi
    Commented Apr 26, 2017 at 14:45
1
\$\begingroup\$

One more tip:


(Object1Property, Object2Property) => new { Object1Property = Object1Property, Object2Property = Object2Property })

Here you don't need to repeat the property names. C# can create them implicitly if their names can/should be the same. This means you can just write:

(Object1Property, Object2Property) => new { Object1Property, Object2Property })
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.