3
\$\begingroup\$

I just faced a problem where I needed to know if extra data was present in a given collection after a Take operation took place. Specifically, this is related to generating @odata.nextLink values in an OData-enabled API only if there is remaining data on the server: the user should not receive a "nextLink" if no more data is available, so that he can properly rely on this value for paging purposes.

The common strategy used in these cases AFAIK is to take one more element on the target, and then check if the results "went past" the limit or not.

For example, when we want to return 10 results:

const int pageSize = 10;
var data = dataSource
    .Take(pageSize + 1)
    .ToList();

var hasRemainingData = data.Count > pageSize;

return new PageResult(
    data: data.Take(pageSize),
    nextLink: hasRemainingData ? CreateNextLink(pageSize) : null);

Now this all felt a bit convoluted to me, so I created an extension method to abstract part of the logic away:

public static (IEnumerable<T> Data, bool HasRemainingData) TakeWithRemainder<T>(this IEnumerable<T> sequence, int count)
{
    if (sequence == null)
        throw new ArgumentNullException(nameof(sequence));

    if (count < 0)
        throw new ArgumentOutOfRangeException(nameof(count));

    var data = sequence.Take(count + 1).ToArray();

    return (new ArraySegment<T>(data, 0, count), data.Length > count);
}

This allows me to clean up the original code significantly, like this:

const int pageSize = 10;
var results = dataSource.TakeWithRemainder(pageSize);

return new PageResult(
    data: results.Data,
    nextLink: results.HasRemainingData ? CreateNextLink(pageSize) : null);

I have a few problems with the extension though, and was wondering if you've got any ideas to make this better:

  1. It materializes the collection

    Not sure if there is a way to avoid this since we need to count the elements anyways, but it sounds unexpected to have a Take overload that materializes the results vs the normal one that does not. Right now it seems like I'm violating the Principle of Least Astonishment here. Should I consider an approach that does not materialize the collection, or should I rename it to something else? Other options?

    .

  2. It relies on ArraySegment to avoid unneeded iteration

    The original code had 2 Take calls in it, which is kinda bad in and of itself. I decided to try using something more decent and went with ArraySegment. Is that intuitive enough to you? I found the code somewhat hard to follow with that in place. Any other options that would still allow me to avoid multiple enumeration are more than welcome.

    .

  3. It uses a value Tuple to get the results out

    Would also want to see your take on this aspect. The named tuple seemed like the most straightforward way to get both the data and the boolean indicator. This of course "breaks" the fluent chain, as you can't immediately chain extra LINQ methods on top of the whole tuple (which could again be seen as a Principle of Least Astonishment violation). Should I consider something else, like a custom iterator class with an extra property that still implemented IEnumerable<T>? That would allow callers to access the boolean, but still chain more LINQ calls as needed.

    At the same time, I wonder if it wouldn't be extra misleading due to the materialization of the collection.

    .

  4. HasRemainingData and TakeWithRemainder seem like poor names to me

    I'm not liking these 2 names, but I'm failing to thinking of something better for them if I am to keep this approach.

\$\endgroup\$
  • \$\begingroup\$ In what context are you using OData? Because Web API has paging built-in: c-sharpcorner.com/article/paging-with-odata-and-Asp-Net-web-api \$\endgroup\$ – BCdotWEB Oct 9 '18 at 9:38
  • \$\begingroup\$ @BCdotWEB yeah, I know. The problem on our side is that we have to control these aspects as we are using Dapper internally tied to stored procedures in the DB. That's why I need to know these details as paging is on our side. I wish we could migrate to EF but that's currently not an option. \$\endgroup\$ – julealgon Oct 9 '18 at 13:37
1
\$\begingroup\$

You should be aware that

return (new ArraySegment<T>(data, 0, count), data.Length > count);

throws an exception if count > data.Count()

The normal Take() doesn't behave that way - it takes min(count, data.Count())


One way to go could be:

public static IEnumerable<T> TakeWithRemainder<T>(this IEnumerable<T> sequence, int count, out bool hasMoreData)
{
  bool moreData = false;

  IEnumerable<T> Iter()
  {
    int i = 0;
    foreach (T value in sequence)
    {
      if (i < count)
      {
        yield return value;
      }
      else
      {
        moreData = true;
        yield break;
      }
      i++;
    }
  }

  var result = Iter().ToList();
  hasMoreData = moreData;
  return result;
}

Another way could be:

public static IEnumerable<T> TakeWithRemainder<T>(this IEnumerable<T> sequence, int count, out bool hasMoreData)
{
  hasMoreData = sequence.Skip(count).Any();
  return sequence.Take(count);
}

Or if you want to take a sub sequence not at the start:

public static IEnumerable<T> TakeWithRemainder<T>(this IEnumerable<T> sequence, int skip, int count, out bool hasMoreData)
{
  sequence = sequence.Skip(skip);
  hasMoreData = sequence.Skip(count).Any();
  return sequence.Take(count);
}

Or if you want to take page number n of m:

public static IEnumerable<T> TakeWithRemainder<T>(this IEnumerable<T> sequence, int page, int count, out bool hasMoreData)
{
  sequence = sequence.Skip(page * count);
  hasMoreData = sequence.Skip(count).Any();
  return sequence.Take(count);
}
\$\endgroup\$
  • 1
    \$\begingroup\$ That was a dumb mistake on my part on the ArraySegment ctor, thanks for pointing that out! As for your suggestions, while I'm not very fond of "duplicating the Take" logic, I'm starting to think that a manual iterator is indeed a lot cleaner and more straighforward than what I came up with. Having said that, I'd really like to avoid the out parameter as it is a known bad practice, but that should be easily doable either with a tuple as in my original code, or with a custom IEnumerable<T> wrapper that exposes it as a property. Marking your reply as the answer for now, thanks again! \$\endgroup\$ – julealgon Oct 9 '18 at 13:51

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.