GET with paging, sorting and filtering

Question

I have a GET method that handles paging, filtering, and sorting. Development has been fairly rapid and I'm not completely sold on some of the implementation. A few assumptions have to be made for this method: I'm using System.Dynamic.Linq. I want to put as much work as possible on SQL Server. The page will make use of both the resulting record set after filtering and paging have been applied, as well as the count of the filtered record set before paging is applied. I have full control of the entity definitions, and each has a [Key] attribute on only one property. Currently, I'm only interested in seeing the IsActive records.

Any thoughts on improvements are welcome. Thanks in advance.

    public IList<TEntity> Get<TParamater>(IList<Expression<Func<TEntity, TParamater>>> includeProperties, out int recordCount, Dictionary<string,string> filter, bool filterExact,
    string orderBy = null, bool orderByAsc = true, int pageNumber = 0, int perPage = 10)

    {
        //IQueryable that will eventually contain the record set
        IQueryable<TEntity> query = _context.Set<TEntity>();
        //IQueryable that will get the count of post-filter, but pre-page records
        IQueryable<TEntity> countQuery = _context.Set<TEntity>();
        List<TEntity> page;


        //countQuery mirrors query through the filter assignments
        query = query.Where("IsActive = true");
        countQuery = countQuery.Where("IsActive = true");

        if (filter.Count > 0)
        {                
            string _template = filterExact ? "{0}.ToString().Equals(@0)" : "{0}.ToString().Contains(@0)";
            string _clause = "";
            foreach (KeyValuePair<string, string> entry in filter)
            {
                _clause = string.Format(_template, entry.Key);
                query = query.Where(_clause, entry.Value);
                countQuery = countQuery.Where(_clause, entry.Value);
            }               
        }
        //countQuery's job is finished at this point
        recordCount = countQuery.Count();

        if (includeProperties != null)
        {
            foreach (var include in includeProperties)
            {

                query = query.Include(include);
            }
        }

        if (orderBy != null)
        {
            query = query.OrderBy(orderBy + ((orderByAsc) ? " ascending" : " descending"));
        }
        else
        {
            //Using reflection to find the [Key] attribute to use as a default ordering because Skip/Take requires an ordered set
            Type entityType = typeof(TEntity);
            var keyField = entityType.GetProperties()
                .Single(p => Attribute.IsDefined(p, typeof(KeyAttribute)));
            query = query.OrderBy(keyField.Name);
        }

        if (pageNumber > 0)
        {
            int _start = (pageNumber - 1) * perPage;
            query = query.Skip(_start);
            query = query.Take(perPage);
        }

        page = query.ToList();

        return page;
    }

Answer 1

//countQuery mirrors query through the filter assignments

I don't think that's necessary. You could have only one query for the first part of the method, then do recordCount = query.Count(); and then keep using query as you do now.

This way, you have less code and less chance of the count getting out of sync.

---

query.Where("IsActive = true")

I realize Dynamic Linq is an option, but I think you shouldn't use it when you don't have to. If possible, I would constraint TEntity to some Entity base class or IEntity interface (maybe you already do that) and then write:

query.Where(entity => entity.IsActive);

---

query = query.Skip(_start);
query = query.Take(perPage);

I don't see any reason why spread this over two lines:

query = query.Skip(_start).Take(perPage);

---

List<TEntity> page;

…

page = query.ToList();

return page;

The variable page does not serve any purpose, get rid of it:

return query.ToList();