HiveBrain v1.2.0
Get Started
← Back to all entries
patterncsharpMinor

Equality Function Generator

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
equalityfunctiongenerator

Problem

I have a small open source library called Equ that I've been using already on several projects. After a rework of the internals I thought I'd ask here for opinions on improvement possibilities.

At the core of my library is a class that generates equality functions, i.e. bool Equals(T a, T b) and int GetHashCode(T obj).

It takes a type and a set of MemberInfo objects that define which members (properties or fields) should be included in the equality functions. The class generates a Func as output for each of Equals() and GetHashCode() using expression trees. The idea behind this approach is that the slow reflection code needs to be run only once per type, using the resulting lambdas as sort of a cache for the code.

Here's the code of the central class of the library, EqualityFunctionGenerator. This is a copy of this.

```
using System;
using System.Collections;
using System.Collections.Generic;
using System.Linq;
using System.Linq.Expressions;
using System.Reflection;

public class EqualityFunctionGenerator
{
private static readonly MethodInfo _objectEqualsMethod = typeof(object).GetMethod("Equals", BindingFlags.Static | BindingFlags.Public);

private readonly Type _type;

private readonly Func> _fieldSelector;

private readonly Func> _propertySelector;

public EqualityFunctionGenerator(Type type, Func> fieldSelector, Func> propertySelector)
{
_type = type;
_fieldSelector = fieldSelector;
_propertySelector = propertySelector;
}

public Func MakeGetHashCodeMethod()
{
var objRaw = Expression.Parameter(typeof(object), "obj");

// cast to the concrete type
var objParam = Expression.Convert(objRaw, _type);

// compound XOR expression
var getHashCodeExprs = GetIncludedMembers(_type).Select(p => MakeGetHashCodeExpression(p.Item1, p.Item2, objParam));
var xorChainExpr = getHashCodeExprs.Aggregate((Expression)Expression.Constant(29), LinkHashCodeExpres

Solution

You should add XML comments, especially for important items (like EqualityFunctionGenerator) and potentially confusing items (like fieldSelector).

typeof(object).GetMethod("Equals", BindingFlags.Static | BindingFlags.Public)


Safer way to do this would be to create a delegate for object.Equals() and then take its Method:

new Func(object.Equals).Method


equalsExprs.Aggregate((Expression)Expression.Constant(true), Expression.AndAlso)


You don't need to use seed here, this will work too:

equalsExprs.Aggregate(Expression.AndAlso)


var objectEqualsExpr = Expression.Equal(leftRaw, rightRaw);


This won't work if the type also overloads == by delegating to Equals() (which is the most reasonable implementation). Instead, I think you should just return false when the types don't match.

Expression.TypeIs(rightRaw, _type)


This doesn't actually check whether the type of rightRaw is _type, it checks whether it's _type or a type derived from _type (just like is does in C#). Here, I think you want to know whether the type is exactly _type. You can do this by calling GetType() (and don't forget to add a null check before doing that).

private static Expression MakeEqualsExpression(MemberInfo member, Type memberType, Expression left, Expression right)


You don't need the memberType parameter here, you can use leftMemberExpr.Type instead.

This means you can get rid of the Tuple in GetIncludedMembers() and return just the MemberInfo.

private static Expression MakeGetHashCodeExpression(MemberInfo member, Type memberType, UnaryExpression obj)


I don't see any reason why obj should be a UnaryExpression, you should use Expression here.

Code Snippets

typeof(object).GetMethod("Equals", BindingFlags.Static | BindingFlags.Public)
new Func<object, object, bool>(object.Equals).Method
equalsExprs.Aggregate((Expression)Expression.Constant(true), Expression.AndAlso)
equalsExprs.Aggregate(Expression.AndAlso)
var objectEqualsExpr = Expression.Equal(leftRaw, rightRaw);

Context

StackExchange Code Review Q#57082, answer score: 3

Revisions (0)

No revisions yet.