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

Indexer of the IEnumerable #1642

Closed
ghost opened this issue Jun 15, 2018 · 16 comments
Closed

Indexer of the IEnumerable #1642

ghost opened this issue Jun 15, 2018 · 16 comments

Comments

@ghost
Copy link

ghost commented Jun 15, 2018

In VB.NET, I can use this code:

Dim result = (From N In {1, -2, 3}
        Where N < 0)

If result.Count() > 0 Then _
    Console.WriteLine(result(0))

The equivalent C# code is:

var result = (from int N in new[] { 1, -2, 3 }
                      where N < 0
                      select N);

if (result.Count() > 0)
    Console.WriteLine(result.ElementAt(0));

Why can I use result(0) in VB but can't use result[0] in C#?
Can this be fixed?

@CyrusNajmabadi
Copy link
Member

CyrusNajmabadi commented Jun 15, 2018

Why can I use result(0) in VB

Because VB allows it:

https://github.com/dotnet/vblang/blob/master/spec/expressions.md#default-query-indexer

but can't use result[0] in C#?

Because C# has no equivalent rule.

Can this be fixed?

The compilers/languages are working as spec'ed. If you have a proposal on a change you'd like to see here, you can def make one :)

Note: my guess is that this would not be accepted. Generally, c# recommends that if you have an indexer it actually operate in O(1) time. In this case, the indexer would actually take O(n) time. So it would not be a good idea to expose this functionality which could lead someone to believe this coudl be done cheaply. Note that VB and C# can have different ideas of what is/isn't ok here. That's why there are different languages in the first place :)

@HaloFour
Copy link
Contributor

Holy crap, I didn't know you could use an IEnumerable(Of T) in VB.NET like that, but that sounds like a really awful idea.

What's worse is that it uses ElementAtOrDefault which would give you 0 even if you index out of the bounds of the enumerable. Terribly inefficient and likely wrong. I can't fathom how that made it into VB.NET.

@ghost
Copy link
Author

ghost commented Jun 15, 2018

@CyrusNajmabadi

which could lead someone to believe this coudl be done cheaply

C# will only compile result[0] to result.ElementAt(0) will it generates IL, so I don't understand what is the cost that can be paid in this.

Thanks.

@CyrusNajmabadi
Copy link
Member

C# will only compile result[0] to result.ElementAt(0)

.ElementAt(n) runs in O(n) time. In general, C# has recommended that indexers run in amortized-constant time (i.e. effectively O(1)).

So this would be a language feature that would literally violate one of the guidelines we have for how the language should behave with APIs.

@ghost
Copy link
Author

ghost commented Jun 15, 2018

@CyrusNajmabadi
Can CoreFx add an Indexer to IEnumerable and other Queyable types?

@CyrusNajmabadi
Copy link
Member

Can CoreFx add an Indexer to IEnumerable and other Queyable types?

You can ask CoreFx that :)

However, my guess would be 'no'. For the same reasons that C# would not want to give it. It would make people think it was fsat/cheap to index into an IEnumerable. And, it's really not.

--

For example, say your stream has 1m+ elements in it. If you access stream[1000000], then that has to iterate all 1m elements. Contrast that with a normal collection that has an indexer. That operation should be sub-linear (and is normally constant).

Having an operator which people thinks runs sublinearly actually be linear is a very big foot-gun.

@CyrusNajmabadi
Copy link
Member

CyrusNajmabadi commented Jun 15, 2018

Note: if we ever get Extension Everything, then you can easily add this yourself by providing an extension indexer on IEnumerable. That way any perf issues can be contained to your own projects. but it's doubtful that C# would ever add this into the language itself.

I also doubt CoreFx would do it. But you're welcome to ask them. However, that should be in their repo, not here.

@ghost
Copy link
Author

ghost commented Jun 15, 2018

@CyrusNajmabadi
I got the idea. Seems that IEnumerable is not truly indexed, but must be iterated from start to end by an an enumerator not a normal indexed for loop. I was thinking of it as a deffered normal collection, and I still can't get why it is not indexed. Is it implemented as a Linked List ?

@ghost
Copy link
Author

ghost commented Jun 15, 2018

@HaloFour

I can't fathom how that made it into VB.NET.

VB.NET is about writting code easily and quickly. But I agree with you that it shout use ElemntAt not ElementAtOrDefault.

@CyrusNajmabadi
Copy link
Member

and I still can't get why it is not indexed. Is it implemented as a Linked List ?

It's an interface. there's no way to know how it's implemented :) For example, here's a totally reasonable implementation of an IEnumerable:

class CyEnumerable : IEnumerable<int>
{
    public IEnumerator<int> GetEnumerator()
    {
        int i = 0;
        while (true)
        {
            yield return i++;
        }
    }

    IEnumerator IEnumerable.GetEnumerator() => GetEnumerator();
}

If you do stream[1000000] it has to enumerate 1m elements. What's worse, if you do:

for (int i = 0; i < whatever; i++)
{
   var v = stream[i];
}

For iteration 5 it will need to enumerate 5 elements. Then on iteration 10 it will need to enumerate 10 elements. Iteration 100 will need to enumerate 100 elements. etc. etc.

So you have:

1 + 
2 + 
3 + 
4 + 
5 + 
...
n

This is equal to n * (n + 1) / 2 (or O(n^2)) actual ops. So just having a linear loop will lead to a polynomial number of actual steps taken. That's a footgun. We don't want those :)

@CyrusNajmabadi
Copy link
Member

I was thinking of it as a deffered normal collection

IEnumerables are just streams of data. For all one knows, they may be infinite (like the example i gave) If you want a (probably) realized, indexable sequence of data, you can just use IReadOnlyList. If you want a definitely realised, indexable sequence of data, just use List.

@HaloFour
Copy link
Contributor

@MohammadHamdyGhanem

Seems that IEnumerable is not truly indexed, but must iterated from start to end by an an enumeator not a normal indexed for loop.

An IEnumerable<T> is just a sequence of values. How those values are stored/generated is completely unknown. It could be a list, or it could be a linked list, or it could be an infinite generator. Moving to the next element could be instant, or it could require some serious number crunching every time you call MoveNext.

VB.NET is about writting code easily and quickly. But I agree with you that it shout use ElemntAt not ElementAtOrDefault.

And correctly. Indexing into an enumerable is not correct. It gives the impression that it can be done easily and efficiently, and that's far from a guarantee that can be made. The fact that it'll then return the default value if you exceed the bounds of the enumeration is just insult to injury. I personally find there to be nothing wrong with using one of LINQ's built-in methods for obtaining a particular value from an enumerable. It makes the developer acknowledge what they're doing.

@CyrusNajmabadi
Copy link
Member

Seems that IEnumerable is not truly indexed, but must iterated from start to end by an an enumeator

Correct. That's a virtue (it allows so many differing implementations and can represent so many types of streams of data), but it's also a drawback (things may be expensive and you dont' know what the actual impl is). If those virtues outweigh the drawbacks for you, then use IEnumerable. If they don't, then use a more appropriate interface/type for your domain specific problem :)

@CyrusNajmabadi
Copy link
Member

VB.NET is about writting code easily and quickly

Yup. So decisions made there may not necessarily apply to C#. C#, in particular, feels like this is a massive footgun that really should be avoided. If people want to index in this manner, just use the actual methods. They're clear and self-documenting and show the user (i.e. you) were totally aware of what you were doing and what sort of behavior/experience you would be getting.

@ghost
Copy link
Author

ghost commented Jun 16, 2018

@CyrusNajmabadi
Thanks. You made it crystal clear :)

If they don't, then use a more appropriate interface/type for your domain specific problem :)

We get IEnumerable from LinQ. I'm not sure this can be changed.

@CyrusNajmabadi
Copy link
Member

I'm not sure this can be changed.

You can convert any result into a value that better fits your domain needs. For example, you can call "ToList" or "ToArray" on a result.

We get IEnumerable from LinQ

Right, because Linq-to-objects works with streams of data (including lazily-created, infinite streams). So, if you're using those, it's best ot be explicit about using .ElementAt so that your code is very clear that it could be doing incredibly expensive stuff. If you want to be able to easily index, then just convert the IEnumerable you get back from linq to another collection type that offers cheap and simple indexing.

@ghost ghost closed this as completed Jun 16, 2018
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants