ardalis / Specification

Base class with tests for adding specifications to a DDD model
MIT License
1.9k stars 241 forks source link

Add Counting Functionality to Docs #135

Open ardalis opened 3 years ago

ardalis commented 3 years ago

See: https://github.com/ardalis/Specification/issues/134#issuecomment-872528093

@fiseni do you think this would be considered a 'base feature'? https://ardalis.github.io/Specification/features/base-features.html

If not where should we describe it? Usage?

fiseni commented 3 years ago

I wouldn't say it's a base feature since the users don't define anything explicitly in the specification. The specification infrastructure enables this feature but ultimately is utilized by the repository implementation.

Description:

  1. All partial evaluators expose information if they should be evaluated for Count operation. Right now it's used only for Count, but in the future might be utilized for something else too, that's why the property is named IsCriteriaEvaluator.
  2. The SpecificationEvaluator accepts an optional parameter that indicates whether all partial evaluators should be used or only the ones flagged as criteria evaluators.
public virtual IQueryable<T> GetQuery<T>(IQueryable<T> query, ISpecification<T> specification, bool evaluateCriteriaOnly = false) where T : class
{
    var evaluators = evaluateCriteriaOnly ? this.evaluators.Where(x => x.IsCriteriaEvaluator) : this.evaluators;

    foreach (var evaluator in evaluators)
    {
        query = evaluator.GetQuery(query, specification);
    }

    return query;
}
  1. The repository implementation simply passes true when evaluates the specification for the CountAsync method.

Ultimately, the users don't have to do anything. They're not forced to define a separate specification just for Count, but they can keep using the same specification, and the rest will be handled by the library. Calling CountAsync is all they need to do. We're excluding the following partial evaluators (features) during this operation

snowfrogdev commented 1 year ago

Ultimately, the users don't have to do anything. They're not forced to define a separate specification just for Count, but they can keep using the same specification, and the rest will be handled by the library. Calling CountAsync is all they need to do. We're excluding the following partial evaluators (features) during this operation

  • Take
  • Skip
  • Ordering
  • Include
  • Select

So does that mean that if I have a specification that uses some of these operations for the actual query logic and not just pagination, this strategy wouldn't work?

Like if I had something like this:

public class PlayersThatAreNotTheBest: Specification<Players>
{
    public PlayersThatAreNotTheBest(int skip, int take)
    {
        Query
          .OrderByDescending(p => p.Goals)
          .Skip(5)

         //paging
         Query
           .Skip(skip)
           .Take(take)
    }
}

Now I know there are easy workarounds to this. For instance in my example you could make the query be for ALL players, ordered by goals, and make the caller specify the "skip 5" as part of his pagination logic on the first call. But I'm just using it for illustration purposes. I'm just wondering if we shouldn't document explicitly the fact that if the users want to use the CountAsync strategy, they have to make sure that they don't use these operations in their query logic, other than for pagination.

I'm brand new to this library so I'm still putting the pieces together. I have a few problems in figuring out how to best document all this. That behavior could be documented at the method level, but the problem is that it is tied to the specific EF implementations of IReadRepositoryBase.CountAsync, not the interface itself. I'm thinking we may have some design decisions to make here. As this comment highlights, it would be nice if we could somehow make this behavior be part of the abstraction and not only specific implementations.

At any rate, if we leave things as is, the only thing that makes sense to me is to document the behavior of CountAsync, as it relates to this discussion, at the method level on the EF RepositoryBase implementations. Adding this behavior to the docs site could be confusing for readers, unless we are very clear that it is a specific behavior of the EF implementations.