patterncsharpMinor
Equality Function Generator
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.
It takes a type and a set of
Here's the code of the central class of the library,
```
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
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
Safer way to do this would be to create a delegate for
You don't need to use
This won't work if the type also overloads
This doesn't actually check whether the type of
You don't need the
This means you can get rid of the
I don't see any reason why
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).MethodequalsExprs.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).MethodequalsExprs.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.