Proposal: construct Range of valid indices for strings and collections

[reima]

Methods working with strings or collections often need to check that an index argument is valid for another string/collection argument.

Currently this would be done in the following way:

index.MustBeIn(Range.FromInclusive(0).ToExclusive(text.Length));

It would be neat if there were methods for constructing a Range of valid indices for a given string/collection, which could shorten the code to something like:

index.MustBeIn(Range.For(text));

This reads similar to Guard.IsInRangeFor from CommunityToolkit.Diagnostics.

The added API surface could look like this:

public static class Range
{
    public static Range<int> For(string text);
    public static Range<int> For(ICollection collection);
    public static Range<int> For<T>(T[] array);
    public static Range<int> For<T>(List<T> list);
    public static Range<int> For<T>(ICollection<T> collection);
    public static Range<int> For<T>(IReadOnlyCollection<T> collection);
    public static Range<int> For<T>(Span<T> span);
    public static Range<int> For<T>(ReadOnlySpan<T> span);
    public static Range<int> For<T>(Memory<T> memory);
    public static Range<int> For<T>(ReadOnlyMemory<T> memory);
}

Note that the overloads for List<T> and T[] are required to resolve ambiguous invocations.

[feO2x]

Sounds good to me. Just one thought: maybe we can use the following API:

public static Range<int> For<TCollection>(TCollection collection) where TCollection : ICollection;

This way we address all regular collections, including Array<T>, List<T>, ObservableCollection<T>, Collection<T>, ImmutableArray<T>, and even things like KeyCollection and ValueCollection of Dictionary<TKey, TValue>.

We then only need overloads for Span (ReadOnlySpan<T> should be enough, Span<T> can be implicitly converted to it), ReadOnlyMemory<T>, and string.

Would you like to contribute this feature to Light.GuardClauses?

[reima]

While there are implicit conversions from Span<T> to ReadOnlySpan<T> and Memory<T> to ReadOnlyMemory<T>, omitting overloads for Span<T> and Memory<T> would require explicitly stating the type parameter: Range.For<bool>(spanOfBool). I believe adding overloads for Span<T> and Memory<T> is not a great deal of work (both implementation and maintenance wise), but would improve developer ergonomics.

The generic For<TCollection> is an interesting solution, which seems to work for most cases. Additional overloads for ICollection<T> and IReadOnlyCollection<T> would still be required to cover these.

Though you mentioning KeyCollection and ValueCollection of Dictionary<TKey, TValue> got me to reconsider if accepting ICollection is a good idea. The intended purpose of Range.For is to construct a valid range of indices for collection types where individual items can be accessed by index. ICollection does not guarantee that the implementing type has an indexer. Indexing an ICollection either requires conversion to a List<T> (or any other type that actually can be indexed) or enumerating e.g. using extension methods like Enumerable.Take<T>(IEnumerable<T>, int). But then the method could just as well either accept an indexable type, or accept an IEnumerable<T>.

The two sensible options in my opinion are:

• Accept types that can be indexed, like IList, IReadOnlyList<T> (+ string, (ReadOnly)Span<T>, (ReadOnly)Memory<T>). This excludes collections like KeyCollection and ValueCollection of Dictionary<TKey, TValue>.
• Accept any IEnumerable (+ string, (ReadOnly)Span<T>, (ReadOnly)Memory<T>).

What do you think?

I would like to contribute this feature, although I'm not sure when I'll get around to it.

[feO2x]

Usually, people expect to simply pass in IEnumerable<T>, no matter what type is exactly used behind the scenes. The LINQ extension methods allow them to do that, so We should probably stick to it as well (although I'm not a big fan of it as it does not express intention, as you also stated).

There is actually an existing method in the code base that we might reuse:

https://github.com/feO2x/Light.GuardClauses/blob/78f583fb00734222db502ac4041d8827184a2bb8/Code/Light.GuardClauses/FrameworkExtensions/EnumerableExtensions.cs#L157

It simply tries to cast to ICollection and to string to obtain the count. If that's not possible, it will enumerate.

[feO2x]

It's a holiday in Germany today, I will take a look at it.

[feO2x]

@reima I think I implemented the feature - check out this PR: https://github.com/feO2x/Light.GuardClauses/pull/94

If you are satisfied with it, we can merge and publish it.

I have overloads for Span<T>, ReadOnlySpan<T>, Memory<T>, and ReadOnlyMemory<T>. All other collection types are addressed with the overload for IEnumerable. While this is not the fastest way to obtain the count of a collection, it still is pretty fast, does not allocate unless the underlying enumerable is lazy, and we support everything that implements IEnumerable.

I'm eager to hear your thoughts.

[reima]

@feO2x Thank you for implementing this feature so quickly!

I've looked at the PR and left some comments there.

[feO2x]

@reima v10.3.0 is now available on NuGet: https://www.nuget.org/packages/Light.GuardClauses/10.3.0

It's been a pleasure working with you. Thank you so much for the proposal and feedback!