I've been experimenting with different ways of reusing (and naming) the expressions I use in Entity Framework. I've tried static fields with Expressions and extension methods but neither are perfect. In my latest attempt to solve this problem, I'm attempting to implement the specification pattern!Firstly, I created a hierarchy for Specifications:Specification.cspublic abstract class Specification<T>{    public abstract Expression<Func<T, bool>> ToExpression();    public abstract bool IsSatisfiedBy(T target);    public static implicit operator Expression<Func<T, bool>>(Specification<T> specification)    {        if (specification == null)        {            throw new ArgumentNullException(nameof(specification));        }        return specification.ToExpression();    }    public Specification<T> And(Specification<T> other)    {        ValidateForCompose(other);        return new AndSpecification<T>(this, other);    }    public Specification<T> Or(Specification<T> other)    {        ValidateForCompose(other);        return new OrSpecification<T>(this, other);    }    public Specification<T> Negate()    {        return new NotSpecification<T>(this);    }    public static Specification<T> operator ! (Specification<T> specification)    {        if (specification == null)        {            throw new ArgumentNullException(nameof(specification));        }        return specification.Negate();    }    private void ValidateForCompose(Specification<T> other)    {        if (ToExpression() == null)        {            throw new InvalidOperationException(                Cannot compose an empty specification with another specification.);        }        if (other == null)        {            throw new ArgumentNullException(nameof(other));        }    }}NotSpecificationpublic class NotSpecification<T> : Specification<T>{    private readonly Specification<T> specification;    public NotSpecification(Specification<T> specification)    {        if (specification == null)        {            throw new ArgumentNullException(nameof(specification));        }        this.specification = specification;    }    public override Expression<Func<T, bool>> ToExpression()    {        return specification.ToExpression().Negate();    }    public override bool IsSatisfiedBy(T target)    {        return !specification.IsSatisfiedBy(target);    }}CompositeSpecificationpublic abstract class CompositeSpecification<T> : Specification<T>{    protected readonly Specification<T>[] Specifications;    protected CompositeSpecification(params Specification<T>[] specifications)    {        if (specifications == null || !specifications.Any())        {            throw new ArgumentNullException(nameof(specifications));        }        Specifications = specifications;    }}AndSpecification public class AndSpecification<T> : CompositeSpecification<T>{    public AndSpecification(params Specification<T>[] specifications)        : base(specifications)    {    }    public override Expression<Func<T, bool>> ToExpression()    {        var firstSpecification = Specifications.First().ToExpression();        if (Specifications.Length == 1)        {            return firstSpecification;        }        return Specifications.Skip(1)            .Aggregate(firstSpecification,                 (current, specification) => current.And(specification.ToExpression()));    }    public override bool IsSatisfiedBy(T target)    {        return Specifications.All(s => s.IsSatisfiedBy(target));    }}OrSpecificationpublic class OrSpecification<T> : CompositeSpecification<T>{    public OrSpecification(params Specification<T>[] specifications)        : base(specifications)    {    }    public override Expression<Func<T, bool>> ToExpression()    {        var firstSpecification = Specifications.First().ToExpression();        if (Specifications.Length == 1)        {            return firstSpecification;        }        return Specifications.Skip(1)            .Aggregate(firstSpecification, (current, specification) => current.Or(specification.ToExpression()));    }    public override bool IsSatisfiedBy(T target)    {        return Specifications.Any(s => s.IsSatisfiedBy(target));    }}ExpressionSpecificationpublic abstract class ExpressionSpecification<T> : Specification<T>{    protected abstract Expression<Func<T, bool>> Expression { get; }    public override Expression<Func<T, bool>> ToExpression()    {        return Expression;    }    public override bool IsSatisfiedBy(T target)    {        return Expression.Compile()(target);    }}There are a couple of Extension methods which make composing the Expressions possible:public static class ExpressionExtensions{    public static Expression<Func<T, bool>> And<T>(this Expression<Func<T, bool>> first, Expression<Func<T, bool>> second)    {        var invokedExpr = Expression.Invoke(second, first.Parameters);        return Expression.Lambda<Func<T, bool>>              (Expression.AndAlso(first.Body, invokedExpr), first.Parameters).ExpandInvocations();    }    public static Expression<Func<T, bool>> Or<T>(this Expression<Func<T, bool>> first, Expression<Func<T, bool>> second)    {        var invokedExpr = Expression.Invoke(second, first.Parameters);        return Expression.Lambda<Func<T, bool>>              (Expression.OrElse(first.Body, invokedExpr), first.Parameters).ExpandInvocations();    }    public static Expression<Func<T, bool>> Negate<T>(this Expression<Func<T, bool>> expr)    {        return Expression.Lambda<Func<T, bool>>(Expression.Not(expr.Body), expr.Parameters).ExpandInvocations();    }    private static Expression<T> ExpandInvocations<T>(this Expression<T> expr)    {        return expr.VisitWith(new InvocationExpander());    }    public static Expression<T> VisitWith<T>(this Expression<T> expression, params ExpressionVisitor[] visitors)    {        return (Expression<T>)((Expression)expression).VisitWith(visitors);    }    public static Expression VisitWith(this Expression expression, params ExpressionVisitor[] visitors)    {        if (expression == null)        {            throw new ArgumentNullException(nameof(expression));        }        return visitors == null            ? expression             : visitors.Aggregate(expression, (current, visitor) => visitor.Visit(current));    }}And finally:// Taken/modified from:// https://blogs.msdn.microsoft.com/meek/2008/12/07/invocationexpression-and-linq-to-entities/public class InvocationExpander : ExpressionVisitor{    private readonly ParameterExpression _parameter;    private readonly Expression _expansion;    private readonly InvocationExpander _previous;    public InvocationExpander() { }    private InvocationExpander(ParameterExpression parameter, Expression expansion, InvocationExpander previous)    {        _parameter = parameter;        _expansion = expansion;        _previous = previous;    }    public InvocationExpander Push(ParameterExpression parameter, Expression expansion)    {        return new InvocationExpander(parameter, expansion, this);    }    protected override Expression VisitInvocation(InvocationExpression iv)    {        if (iv.Expression.NodeType != ExpressionType.Lambda)        {            return base.VisitInvocation(iv);        }        var lambda = (LambdaExpression)iv.Expression;        return lambda            .Parameters            .Select((x, i) => new { Parameter = x, Expansion = iv.Arguments[i] })            .Aggregate(this, (previous, pair) => previous.Push(pair.Parameter, pair.Expansion))            .Visit(lambda.Body);    }    protected override Expression VisitParameter(ParameterExpression p)    {        var expander = this;        while (null != expander)        {            if (expander._parameter == p)            {                return base.Visit(expander._expansion);            }            expander = expander._previous;        }        return base.VisitParameter(p);    }}All of that allows me to do something like:public class IsPublic : ExpressionSpecification<Posting>{    private static readonly Expression<Func<Posting, bool>> IsPublicExpression =        p => p.VisibilityId == PostingVisibilityId.Public;    protected override Expression<Func<Posting, bool>> Expression => IsPublicExpression;}The real expressiveness comes from being able to make lots of small Specifications which are then either composed using And or Or or by creating reusable CompositeExpressions:public class IsAnIncompleteChange : AndSpecification<Response>{    public IsAnIncompleteChange()        : base(            new IsPublic(),            new LinkedPostingIsPublic(),            new IsFromASubscription(),            new IsPlanningChange(),            new SubsequentChangeHasNotBeenMade())    {    }}Both of those are examples from the real application that I'm trying this out on. So why am I trying to do this?Up until now we've had extension methods for reusable predicates which looks nice but I found that the generated SQL (when things get complex with projections and joins) sometimes repeats predicates in sub queries when we were doing things like:// postings is an IQueryablepostings.WhichArePublic()        .WhichAreSignedPersonally()        .WhichAreInResponseToACriticalPosting()        .ProjectIntoFeaturedPostingModel();By creating specifications instead I've been able to quite dramatically decrease the cost of several of our more expensive queries (up to 70% reduction in reads and elapsed time). Not to mention more readable queries:postings.Where(new HasPublicPersonalResponseToCriticalPosting());The limitation with the above is that you can't use them within an Expression e.g. postings.Select(p => new { Responses = p.Responses.AsQueryable().Where(new IsPublic()) });Which is where part 2 comes in...So, can this code be improved? Is there a better way of reusing predicates and having modular and well named predicates?Have I made any glaring coding (or style) mistakes?
Specification pattern in EF part 1
c#;entity framework;expression trees
I might be missing something, but from what I can see IsSatisfiedBy is only ever used by IsSatisfiedBy, and can be removed.Consider making ToExpression protected, since with the implicit operator I don't see how exposing it is helpful.Consider exposing operators & and | to make composition of more complicated specifications easier to read and write.Here's an illustration of how I'm imaginging it:public abstract class Specification<T>{    protected abstract Expression<Func<T, bool>> Expression { get; }    public static implicit operator Expression<Func<T, bool>>(Specification<T> specification)    {        if (specification == null)        {            throw new ArgumentNullException(nameof(specification));        }        return specification.Expression;    }    public Specification<T> And(Specification<T> second)    {        if (second == null)        {            throw new ArgumentNullException(nameof(second));        }        return new AndSpecification(this, second);    }    public static Specification<T> operator &(Specification<T> first, Specification<T> second)    {        if (first == null)        {            throw new ArgumentNullException(nameof(first));        }        if (second == null)        {            throw new ArgumentNullException(nameof(second));        }        return new AndSpecification(first, second);    }    private sealed class AndSpecification : Specification<T>    {        public AndSpecification(Specification<T> first, Specification<T> second)        {            Expression = first.Expression.And(second);        }        protected override Expression<Func<T, bool>> Expression { get; }    }    // Similarly for or, not.}And example client codepublic class IsSomeExample : Specification<Posting>{    protected override Expression<Func<Posting, bool>> Expression =>        new IsPrivate() | (new IsFromASubscription() & new LinkedPostingIsPublic());}