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]: Add IEnumerator.CanReset #103855

Closed
koszeggy opened this issue Jun 22, 2024 · 2 comments
Closed

[API Proposal]: Add IEnumerator.CanReset #103855

koszeggy opened this issue Jun 22, 2024 · 2 comments
Labels
api-suggestion Early API idea and discussion, it is NOT ready for implementation area-System.Linq

Comments

@koszeggy
Copy link

koszeggy commented Jun 22, 2024

Background and motivation

Now that collection interfaces are being rationalized (even though the first attempt has been reverted) I would like to suggest a small fix that bothers me since .NET Framework 2.0, when yield return-like iterators rendered the IEnumerator.Reset() method unusable. The issue could have been mitigated by adding a bool CanReset property to the new generic IEnumerator<T> interface but it missed the opportunity.

I know that Reset() mainly exists to support COM compatibility and is not needed normally but as I created a lot of collections with custom enumerators it annoys me a bit that I usually implement the Reset() method but as there is no way to query whether it's usable it's better to avoid it anyway. The best I could do is just to mention everywhere in the documentation whether a collection's enumerator does or does not support the Reset() method, which is obviously a useless information when just handling the instance as a random IEnumerator.

API Proposal

Considering that also #31001 suggests using some default interface implementations, I also dare to suggest the following fix:

public interface IEnumerator
{
-   void Reset();
+   void Reset() => throw new NotSupportedException("To support the Reset() method implement it along with the CanReset property");
+   bool CanReset => false;
}

Please note that the default implementation is compatible with the yield return approach and does not force you to implement the Reset() method for explicit enumerators either.

API Usage

public class MyFancyEnumerator<T> : IEnumerator<T>
{
    private int _someSelfIndex;
    private IEnumerable<T> _someCollection;
    private IEnumerator<T> _someWrappedEnumerator;

    // [...]

    public void Reset()
    {
        _someSelfIndex = -1;
        if (_someWrappedEnumerator.CanReset)
            _someWrappedEnumerator.Reset();
        else // allocating a new instance
           _someWrappedEnumerator = _someCollection.GetEnumerator();
    }
}

Risks

Just by introducing this change all existing enumerators would return false for CanReset, even if they actually support it. Which actually isn't a breaking change; it just reflects the way we should consider Reset() today.

A future version of the .NET Analyzers simply could emit a warning for explicitly implemented enumerators if their Reset() method consists of something else than just throwing a NotSupportedException.

@koszeggy koszeggy added the api-suggestion Early API idea and discussion, it is NOT ready for implementation label Jun 22, 2024
@dotnet-policy-service dotnet-policy-service bot added the untriaged New issue has not been triaged by the area owner label Jun 22, 2024
Copy link
Contributor

Tagging subscribers to this area: @dotnet/area-system-linq
See info in area-owners.md if you want to be subscribed.

@koszeggy koszeggy changed the title [API Proposal]: Add IEnumerable.CanReset [API Proposal]: Add IEnumerator.CanReset Jun 22, 2024
@eiriktsarpalis
Copy link
Member

Introducing DIMs to existing interface types is something we have been wanting to do for a long time, but it's something that we simply haven't been able to find success in so far. The simple reason is that it's a type of change that fundamentally introduces all sorts of breaking changes and disruption, and this is doubly true for extremely popular types like collection interfaces. If we ever get around to making another attempt it would most likely be focusing on highly impactful improvements.

The case of Enumerator.Reset() is frankly not particularly impactful. It's largely a legacy API that newer collections don't implement and can easily be worked around by simply creating a new enumerator.

@eiriktsarpalis eiriktsarpalis closed this as not planned Won't fix, can't repro, duplicate, stale Jul 5, 2024
@dotnet-policy-service dotnet-policy-service bot removed the untriaged New issue has not been triaged by the area owner label Jul 5, 2024
@github-actions github-actions bot locked and limited conversation to collaborators Aug 5, 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.Linq
Projects
None yet
Development

No branches or pull requests

2 participants