-
-
Notifications
You must be signed in to change notification settings - Fork 217
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Version 10 #276
Version 10 #276
Conversation
src/X.PagedList/OrderedPagedList.cs
Outdated
{ | ||
InitSubset(superset, keySelector.Compile(), pageNumber, pageSize); | ||
} | ||
InitSubset(superset, keySelector.Compile(), pageNumber, pageSize); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note that there isn't really much point to creating a PagedList without a superset.
Personally, I would make the superset argument here non-nullable, throw an exception if it is, and drop the nullable handling after that point.
If somebody really wants a PagedList that is empty, he could still supply an empty list to the constructor. This would look like this:
public PagedList(IQueryable<T> superset, Expression<Func<T, TKey>> keySelector, int pageNumber, int pageSize)
: base(pageNumber, pageSize, superset.Count())
{
if (superset == null)
throw new ArgumentNullException(nameof(superset));
// add items to internal list
if (TotalItemCount > 0)
{
InitSubset(superset, keySelector.Compile(), pageNumber, pageSize);
}
}
I would do the same to equivalent cases, i.e. never allow null for the superset anywhere.
This also means that you do not have to change InitSubset at all.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agree
I'll prepare two commits for nullable changes as suggested above and for the direct Subset assignment. |
# Conflicts: # src/X.PagedList/PagedList.cs
No description provided.