Skip to content
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

API Proposal: Range.Contains(Index) #417

Closed
am11 opened this issue Dec 1, 2019 · 7 comments
Closed

API Proposal: Range.Contains(Index) #417

am11 opened this issue Dec 1, 2019 · 7 comments
Labels
api-suggestion Early API idea and discussion, it is NOT ready for implementation area-System.Runtime

Comments

@am11
Copy link
Member

am11 commented Dec 1, 2019

Given a range

[start..end]

in order to check if a number X is within this range, one needs to remember that start of the range is inclusive and end is exclusive. Also, the end value can be greater than start. It would be helpful if Contains(Index) is added either as a first-class or an extension method in Range class:

public class Range
{
    public bool Contains(Index subject) => Start.Value < End.Value ? 
        subject.Value >= Start.Value && subject.Value < End.Value :
        false;
}

Inspired by:

@Dotnet-GitSync-Bot Dotnet-GitSync-Bot added the untriaged New issue has not been triaged by the area owner label Dec 1, 2019
@stephentoub
Copy link
Member

stephentoub commented Dec 1, 2019

How would an API like that work for something like:

[1..3].Contains(^1)

where it doesn't have enough information to produce an answer? Where/how would you use this method?

@deadpaukevich
Copy link

deadpaukevich commented Dec 1, 2019 via email

@am11
Copy link
Member Author

am11 commented Dec 1, 2019

The intended usage is to do a bound check using Range type: (lowerBound..upperBound).Contains(myValue).
In this case, the index from end (^) operator would use the upper bound (End) of Range object:

(1..3).Contains(^0) => false
(1..3).Contains(^1) => true
(1..3).Contains(^2) => true
(1..3).Contains(^3) => false
(1..3).Contains(^4) => false

@huoyaoyuan
Copy link
Member

(1..3).Contains(^0) => false
(1..3).Contains(^1) => true
(1..3).Contains(^2) => true
(1..3).Contains(^3) => false
(1..3).Contains(^4) => false

This is not how range and index work.
To be meaningful, range.Contains(index) should means collection[range] contains collection[index]. The result is relative to collection.Length.

@stephentoub
Copy link
Member

stephentoub commented Dec 1, 2019

This is not how range and index work.
To be meaningful, range.Contains(index) should means collection[range] contains collection[index]. The result is relative to collection.Length.

Right. That's why I was asking how you expect such an API to be used. I'd expect the intended usage to be in the implementation of other types, and in such types it is likely going to be wrong.

@am11
Copy link
Member Author

am11 commented Dec 1, 2019

This proposal is for "is the given index within the range object", and not for "is the given index within the collection object". I understand that thus far, Range is mainly perceived for indexers and slice related use cases. However, Range (as an independent) type can have useful utility methods for more general use cases.

how you expect such an API to be used

I am using it to validate values read in web API:

var index = ReadIndex();
var range = ReadRange();

// validate
if (!RangeContains(range, index))
{
   return Error("index was out of range"); 
}

..

@stephentoub
Copy link
Member

stephentoub commented Dec 1, 2019

Thanks. I think that should not be part of Range. It's arguably changing the expected meaning of Index, and I expect it's likely to do more harm (from being misused) than good.

@jkotas jkotas added area-System.Runtime api-suggestion Early API idea and discussion, it is NOT ready for implementation labels Dec 2, 2019
@ghost ghost locked as resolved and limited conversation to collaborators Dec 11, 2020
@tannergooding tannergooding removed the untriaged New issue has not been triaged by the area owner label Jun 24, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
api-suggestion Early API idea and discussion, it is NOT ready for implementation area-System.Runtime
Projects
None yet
Development

No branches or pull requests

8 participants