Repository navigation
Proposal: construct Range of valid indices for strings and collections #93
Description
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.
Activity
feO2x commentedon Sep 28, 2023
Sounds good to me. Just one thought: maybe we can use the following API:
This way we address all regular collections, including
Array<T>,List<T>,ObservableCollection<T>,Collection<T>,ImmutableArray<T>, and even things likeKeyCollectionandValueCollectionofDictionary<TKey, TValue>.We then only need overloads for Span (
ReadOnlySpan<T>should be enough,Span<T>can be implicitly converted to it),ReadOnlyMemory<T>, andstring.Would you like to contribute this feature to Light.GuardClauses?
reima commentedon Sep 29, 2023
While there are implicit conversions from
Span<T>toReadOnlySpan<T>andMemory<T>toReadOnlyMemory<T>, omitting overloads forSpan<T>andMemory<T>would require explicitly stating the type parameter:Range.For<bool>(spanOfBool). I believe adding overloads forSpan<T>andMemory<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 forICollection<T>andIReadOnlyCollection<T>would still be required to cover these.Though you mentioning
KeyCollectionandValueCollectionofDictionary<TKey, TValue>got me to reconsider if acceptingICollectionis a good idea. The intended purpose ofRange.Foris to construct a valid range of indices for collection types where individual items can be accessed by index.ICollectiondoes not guarantee that the implementing type has an indexer. Indexing anICollectioneither requires conversion to aList<T>(or any other type that actually can be indexed) or enumerating e.g. using extension methods likeEnumerable.Take<T>(IEnumerable<T>, int). But then the method could just as well either accept an indexable type, or accept anIEnumerable<T>.The two sensible options in my opinion are:
IList,IReadOnlyList<T>(+string,(ReadOnly)Span<T>,(ReadOnly)Memory<T>). This excludes collections likeKeyCollectionandValueCollectionofDictionary<TKey, TValue>.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 commentedon Sep 29, 2023
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:
Light.GuardClauses/Code/Light.GuardClauses/FrameworkExtensions/EnumerableExtensions.cs
Line 157 in 78f583f
It simply tries to cast to
ICollectionand tostringto obtain the count. If that's not possible, it will enumerate.feO2x commentedon Oct 3, 2023
It's a holiday in Germany today, I will take a look at it.
feO2x commentedon Oct 3, 2023
@reima I think I implemented the feature - check out this PR:
#94
If you are satisfied with it, we can merge and publish it.
I have overloads for
Span<T>,ReadOnlySpan<T>,Memory<T>, andReadOnlyMemory<T>. All other collection types are addressed with the overload forIEnumerable. 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 implementsIEnumerable.I'm eager to hear your thoughts.
reima commentedon Oct 4, 2023
@feO2x Thank you for implementing this feature so quickly!
I've looked at the PR and left some comments there.
feO2x commentedon Oct 9, 2023
@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!