Problem
I have created one generic Page
class for store paging information with its data.
The class look like:
public class Page<T>
where T : new()
{
private List<T> _list = new List<T>();
[JsonProperty(PropertyName = "total_count")]
public long TotalCount { get; set; }
[JsonProperty(PropertyName = "page_start")]
public long PageStart { get; set; }
[JsonProperty(PropertyName = "page_size")]
public long PageSize { get; set; }
[JsonProperty(PropertyName = "list")]
public List<T> List
{
get { return _list; }
set { _list = value; }
}
}
For every table model of my application uses generic Page
class in repository for return PageSize
and its total records with its List
.
My repository use like below:
public class StudentRepository
{
private readonly CountingCloud_DevContext _dbContext;
public StudentRepository(CountingCloud_DevContext dbContext)
{
_dbContext = dbContext;
}
public Page<StudentResponse> Get(Dictionary<string, object> filterParameters)
{
var resultfinal = new Page<StudentResponse>();
int from = 0, size = 10;
resultfinal = Paging(resultfinal, filterParameters, ref from, ref size);
var query = _dbContext.Students.ToList();
var result = query.Select(x => new StudentResponse
{
Id = x.Id,
Name = x.Name
});
resultfinal.TotalCount = result.Count();
query = result.Skip(from).Take(size);
return resultfinal;
}
private static Page<StudentResponse> Paging(Page<StudentResponse> pagingResponse, Dictionary<string, object> parameters, ref int from, ref int size)
{
if (parameters != null)
{
if (parameters.ContainsKey("PageSize") && parameters.ContainsKey("PageStart"))
{
if (Convert.ToInt32(parameters["PageSize"]) != 0)
{
size = Convert.ToInt32(parameters["PageSize"]);
from = (Convert.ToInt32(parameters["PageStart"]) - 1) * size;
pagingResponse.PageSize = size;
pagingResponse.PageStart = Convert.ToInt32(parameters["PageStart"]);
}
}
else
{
pagingResponse.PageSize = size;
pagingResponse.PageStart = from + 1;
}
}
else
{
pagingResponse.PageSize = size;
pagingResponse.PageStart = from + 1;
}
return pagingResponse;
}
}
Currently Paging()
method I have use for all table like my other Repository is for Employee then for that paging method is look like below:
private static Page<EmployeeResponse> Paging(Page<StudentResponse> pagingResponse, Dictionary<string, object> parameters, ref int from, ref int size)
{
if (parameters != null)
{
if (parameters.ContainsKey("PageSize") && parameters.ContainsKey("PageStart"))
{
if (Convert.ToInt32(parameters["PageSize"]) != 0)
{
size = Convert.ToInt32(parameters["PageSize"]);
from = (Convert.ToInt32(parameters["PageStart"]) - 1) * size;
pagingResponse.PageSize = size;
pagingResponse.PageStart = Convert.ToInt32(parameters["PageStart"]);
}
}
else
{
pagingResponse.PageSize = size;
pagingResponse.PageStart = from + 1;
}
}
else
{
pagingResponse.PageSize = size;
pagingResponse.PageStart = from + 1;
}
return pagingResponse;
}
I would like make this private method is generic so all repository use same,
please suggest me what I am implemented is correct or any better improvement for code any changes.
Solution
Some quick remarks:
-
Names shouldn’t contain underscores etc:
CountingCloud_DevContext
. (The exception is the underscore at the start of a private member (e.g._dbContext
.) -
If you have a Dictionary, do not use
ContainsKey
if you need to retrieve the value associated with that key. This is why the much more efficientTryGetValue
exists. -
Do not copy-paste magic strings (e.g. “PageSize”) all over the place. Store them as
const
in a relevant class and use that. -
But most importantly: what’s the point of
Dictionary<string, object>
when all it contains is two specific key-value pairs? Why not make aPagingParameters
class withPageSize
andPageStart
as nullableint
s? Don’t make things too generic, because all you end up is making things harder for yourself. -
Also avoid making a generic Repository class. It likely will create more work and solve nothing.
The Page class does not need to have this T:new constraint, and the List can be an automatic property, just as the other Properties are.
The Dictionary parameter can be changed to an IReadOnlyDictionary, to indicate, your methods only read the dictionary,
they don’t modify anything.
The Paging method takes a Page, and returns the same Page ?
If there is no code, that eventually changes the Page to something else, the return value is not necessary.
You can consider making your Paging-method a part of the Page class itself.
pagingResponse.Paging(filterParameters, ref from, ref size)
may look simpler.
If you only Count something from a Query in result.Count(); why do you do a complicated select before ?
The result of this
var query = _dbContext.Students.ToList();
var result = query.Select(x => new StudentResponse
{
Id = x.Id,
Name = x.Name
});
resultfinal.TotalCount = result.Count();
Is exactly the same as from
_dbContext.Students.Count();
Your last query is kind of useless.
query = result.Skip(from).Take(size);
It’s created but not executed, and thrown away at the end of the routine.
And this makes the from and size parameters useless itself, they aren’t good for anything.
You have the information of “size” and “from” already availabel in PageSize and PageStart.
Why passing it out of the Paging method a second time as a ref parameter ?
There might be some iteration missing, you seem to create one page only – and List is unassigned.