Skip to content

Commit

Permalink
X.PagedList: PagedListExtensions: Fix logic in ToPagedList()
Browse files Browse the repository at this point in the history
The command flow in ToPagedList with totalSetCount appears odd:

  * For totalCount <= 0, we should simply return an empty list

  * For supersetCount <= pageSize, we should not need additional conditions
    to simply read the list

  * From totalSetCount.HasValue, we know whether to use Count(), but not whether
    or not to use Skip/Take

  * Count() is always executed, even if we supply totalSetCount, which will
    create useless iterations and/or DB roundtrips if we already know the size.

As a consequence, it appears more appropriate to use the logic from
X.PagedList.EF here as well (only in the sync version), which addresses all the
mentioned issues.

Fixes: #266
  • Loading branch information
adschmu committed Jul 9, 2024
1 parent 4e2f214 commit 35a4cc9
Showing 1 changed file with 8 additions and 15 deletions.
23 changes: 8 additions & 15 deletions src/X.PagedList/PagedListExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ public static IPagedList<TResult> Select<TSource, TResult>(this IPagedList<TSour
/// <param name="superset">
/// The collection of objects to be divided into subsets. If the
/// collection implements <see cref="IQueryable{T}"/>, it will be treated as such.
/// </param>
/// </param>
/// <returns>A subset of this collection of objects that can be individually accessed by index and containing
/// metadata about the collection of objects the subset was created from.</returns>
/// <seealso cref="PagedList{T}"/>
Expand Down Expand Up @@ -159,7 +159,7 @@ public static IPagedList<T> ToPagedList<T>(this IEnumerable<T> superset, int pag
/// <returns>
/// A subset of this collection of objects that can be individually accessed by index and containing metadata
/// about the collection of objects the subset was created from.
/// </returns>
/// </returns>
public static IPagedList<T> ToPagedList<T, TKey>(this IEnumerable<T> superset, Expression<Func<T, TKey>> keySelector, int pageNumber, int pageSize)
{
if (superset == null)
Expand Down Expand Up @@ -204,27 +204,20 @@ public static IPagedList<T> ToPagedList<T>(this IQueryable<T> superset, int page
throw new ArgumentOutOfRangeException($"pageSize = {pageSize}. PageSize cannot be less than 1.");
}

var supersetCount = superset.Count();

if (supersetCount > totalSetCount)
{
throw new ArgumentOutOfRangeException($"superset count = {supersetCount} superset count cannot be more than {totalSetCount.Value}.");
}
int totalCount = totalSetCount ?? superset.Count();

List<T> subset;

var totalCount = totalSetCount ?? supersetCount;

if ((totalCount <= 0 || totalSetCount.HasValue) && supersetCount <= pageSize)
{
subset = superset.ToList();
}
else
if (totalCount > 0)
{
var skip = (pageNumber - 1) * pageSize;

subset = superset.Skip(skip).Take(pageSize).ToList();
}
else
{
subset = new List<T>();
}

return new StaticPagedList<T>(subset, pageNumber, pageSize, totalCount);
}
Expand Down

0 comments on commit 35a4cc9

Please sign in to comment.