patterncsharpMinor
GET with paging, sorting and filtering
Viewed 0 times
sortingpagingwithgetandfiltering
Problem
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 Get(IList>> includeProperties, out int recordCount, Dictionary filter, bool filterExact,
string orderBy = null, bool orderByAsc = true, int pageNumber = 0, int perPage = 10)
{
//IQueryable that will eventually contain the record set
IQueryable query = _context.Set();
//IQueryable that will get the count of post-filter, but pre-page records
IQueryable countQuery = _context.Set();
List 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 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 i
Any thoughts on improvements are welcome. Thanks in advance.
```
public IList Get(IList>> includeProperties, out int recordCount, Dictionary filter, bool filterExact,
string orderBy = null, bool orderByAsc = true, int pageNumber = 0, int perPage = 10)
{
//IQueryable that will eventually contain the record set
IQueryable query = _context.Set();
//IQueryable that will get the count of post-filter, but pre-page records
IQueryable countQuery = _context.Set();
List 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 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 i
Solution
//countQuery mirrors query through the filter assignmentsI 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 page;
…
page = query.ToList();
return page;The variable
page does not serve any purpose, get rid of it:return query.ToList();Code Snippets
//countQuery mirrors query through the filter assignmentsquery.Where("IsActive = true")query.Where(entity => entity.IsActive);query = query.Skip(_start);
query = query.Take(perPage);query = query.Skip(_start).Take(perPage);Context
StackExchange Code Review Q#133200, answer score: 4
Revisions (0)
No revisions yet.