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

Add Enumerable.IsEmpty that judges IEnumerable<T> is empty. #28610

Closed
RyotaMurohoshi opened this issue Feb 2, 2019 · 29 comments
Closed

Add Enumerable.IsEmpty that judges IEnumerable<T> is empty. #28610

RyotaMurohoshi opened this issue Feb 2, 2019 · 29 comments
Labels
api-suggestion Early API idea and discussion, it is NOT ready for implementation area-System.Linq
Milestone

Comments

@RyotaMurohoshi
Copy link

Summary

Add IsEmpty method.

  • Judges that IEnumerable<T> is empty.
  • IEnumerable<T> extension method. New LINQ to Objects member.
  • Someone uses "Count() == 0" incorrectly. IsEmpty method helps them.

API

public static class Enumerable
{
    public static bool IsEmpty<TSource>(this IEnumerable<TSource> source);
}

Discussion [UPADTED]

How about use !enumrable.Any()?

I think enumerable.IsEmpty() is better than !enumerable.Any() with next reasons.

  • IsEmpty() is more clear and more explicit to check that enumerable is empty than !enumerable.Any().
  • For C# beginner, !enumerable.Any() to check empty is not familiar, not clear and not easy to find.

I think everyone who participates in this issue is familiar to C# and LINQ. So we know how to use !enumerable.Any() and reason to use !enumerable.Any().

But I think that C# beginner is not same. Please imagine C# beginner. It is a little difficult to find usage of !enumerable.Any() to check empty for them. I think IsEmpty is so easy to find and useful to them.

Background

There are no method or property to judge that Array, IEnumerable<T> and List<T> etc are empty or not.

Someone uses "Count() == 0" incorrectly to judge that IEnumerable<T> is empty or not. This is bad usage. Next code is an example.

Next code read all line twice. To judge target.txt is empty, we do not need read all lines. If target.txt is huge, it is so bad.

IEnumerable<string> lines = File.ReadLines("target.txt");
// Bad solution
if(lines.Count() == 0) {
    Console.WriteLine("target.txt is empty.");
    return ;
}

// iterate all line and use it.

Good practice is to use Any method and ! to judge is empty. This can judge that file is empty or not without reading all lines.

IEnumerable<string> lines = File.ReadLines("target.txt");
// Current good solution
if(!lines.Any()) {
    Console.WriteLine("target.txt is empty.");
    return ;
}

// iterate all line and use it.

By the way, I do NOT think this is BEST way.

Solution and Usage

I suggest IsEmpty method to IEnumerable<T> as extension method. IsEmpty judeges that Array, IEnumerable<T> and List<T> etc are empty or not.

IEnumerable<string> lines = File.ReadLines("target.txt");
if(lines.IsEmpty()) {
    Console.WriteLine("target.txt is empty.");
    return ;
}

// iterate all line and use it.

This is more clear and explicit than Any and ! usage. And beginner can notice the IsEmpty method with IntelliSense.

Relatives

There is IsEmpty method in Ix.NET.

Ix.NET

https://github.com/dotnet/reactive/blob/master/Ix.NET/Source/System.Interactive/Empty.cs

Next languages have IsEmpty or isEmpty methods.

F#
https://msdn.microsoft.com/en-us/visualfsharpdocs/conceptual/list.isempty%5B't%5D-function-%5Bfsharp%5D
https://msdn.microsoft.com/en-us/visualfsharpdocs/conceptual/array.isempty%5b't%5d-function-%5bfsharp%5d
https://msdn.microsoft.com/en-us/visualfsharpdocs/conceptual/seq.isempty%5b't%5d-function-%5bfsharp%5d

Java
https://docs.oracle.com/javase/10/docs/api/java/util/Collection.html#isEmpty()

Kotlin
https://kotlinlang.org/api/latest/jvm/stdlib/kotlin.collections/is-empty.html
https://kotlinlang.org/api/latest/jvm/stdlib/kotlin.collections/-collection/is-empty.html

Scala
https://www.scala-lang.org/api/current/scala/collection/GenTraversableOnce.html#isEmpty:Boolean

Update

Add Discussion : How about use !enumrable.Any()?

@Wraith2
Copy link
Contributor

Wraith2 commented Feb 3, 2019

Why not just use !enumerable.Any()

@RyotaMurohoshi
Copy link
Author

RyotaMurohoshi commented Feb 3, 2019

Why not just use !enumerable.Any()

Nice question!

The reason why I think enumerable.IsEmpty() is better than !enumerable.Any() are

  • IsEmpty() is more clear and more explicit to check that enumerable is empty than !enumerable.Any().
  • For C# beginner, !enumerable.Any() to check empty is not familiar, not clear and not easy to find.

I think everyone who participates in this issue is familiar to C# and LINQ.
So we know how to use !enumerable.Any() and reason to use !enumerable.Any().

But I think that C# beginner is not same.
Please imagine C# beginner. It is a little difficult to find usage of "!enumerable.Any()" to check empty for them.

I think IsEmpty is so easy to find and useful to them.

C# and .NET core is not for only current C#er. I think IsEmpty is so good and helpful for future C# and .NET core user.

@RyotaMurohoshi
Copy link
Author

Thank you for your comment. I think other people have same opinion with you.
https://github.com/dotnet/corefx/issues/35054#issuecomment-460042178

So I update and add the next answer content on top issue comment.
https://github.com/dotnet/corefx/issues/35054#issuecomment-460056369

@Gnbrkm41
Copy link
Contributor

Gnbrkm41 commented Feb 3, 2019

Any() and its overloads seem to be more versatile than IsEmpty (if there was one) method, though I do agree that IsEmpty would be much simpler and easier to understand/read.

Has there been any case where a new method was added in BCL/FCL that simply call other methods for clarity's sake? (Basically renaming an existing one)

@Zenexer
Copy link
Contributor

Zenexer commented Feb 5, 2019

Adding IsEmpty won’t allow you to iterate over the items. It will have the same issue as Any() and Count(). Any time you read from an IEnumerable, that’s it—you can’t enumerate it again. That includes Any and Count, and it would also include IsEmpty.

Your argument is that adding IsEmpty would make correct usage clearer than Any, but your usage of Any is incorrect in the same way as your example of Count. If IsEmpty gives you the impression that you can reuse the IEnumerable, then it’s misleading.

// iterate all line and use it.

Maybe, or maybe not. Maybe it skips the first line, maybe it throws an exception. It might work, but that’s not guaranteed.

@Gnbrkm41
Copy link
Contributor

Gnbrkm41 commented Feb 5, 2019

@Zenexer I don't understand what you're saying. Do you mean IEnumerator<T>? If that's the case, yes it is supposed to be used once. but still, you can call GetEnumerator() on IEnumerable<T> as many times as you want to get a new one?

Example:

using System;
using System.Collections.Generic;

namespace ConsoleApp10 //:^)
{
    class Program
    {
        static void Main(string[] args)
        {
            List<int> list = new List<int> { 1, 2, 3, 4, 5, 6, 7, 8, 9, 10 };

            WriteAllIfNotEmpty(list);
        }

        public static void WriteAllIfNotEmpty<T>(IEnumerable<T> enumerable)
        {
            if (!enumerable.IsEmpty()) //Using the enumerable
            {
                foreach (T item in enumerable)
                {
                    Console.WriteLine(item);
                    //Would it really work here, given that I've used enumerable already?
                }
            }
            else
                Console.WriteLine("Oops, empty");
        }
    }

    static class Extensions
    {
        public static bool IsEmpty<T>(this IEnumerable<T> enumerable)
        {
            if (enumerable is null) throw new ArgumentNullException(nameof(enumerable));

            using (IEnumerator<T> enumerator = enumerable.GetEnumerator())
                return !enumerator.MoveNext();
        }
    }
}

Outputs:

1
2
3
4
5
6
7
8
9
10

@MarkusAmshove
Copy link

In my opinion this would help beginners a lot. I often see Count() == 0 and people don't really learn to use Any until an analyzer tells them

@vcsjones
Copy link
Member

vcsjones commented Feb 6, 2019

This seems to be a duplicate suggestion of #27661 albeit with a different name.

@kashifasif
Copy link

@RyotaMurohoshi You are right. The problem with DataObject.count() == 0 is that when DataObject is NULL Then it will throw exception. As i know we can handle it with this way DataObject?.count() which will not throw exception. but in my openion DataObject.IsEmpty() is more straight forward for checking.

@Gnbrkm41
Copy link
Contributor

Gnbrkm41 commented Feb 7, 2019

@kashifasif IEnumerable.Empty is not the same as null; the suggested problem with Count() is that it actually has to enumerate through the enumerable to count how many items are in the enumerable, which is unnecessary if we just want to check if the enumerable is empty or not. (imagine the resource wasted to enumerate through 2^31-1 items, when what we really need to know is if there's more than one item or not)

Although it is an extension method (therefore legal to call on (IEnumerable<T>)null, unlike instance methods), I think it should not replace the null check and explicit null check (enumerable is null) must be used.

@RyotaMurohoshi
Copy link
Author

Hi @vcsjones !

This seems to be a duplicate suggestion of #27661 albeit with a different name.

in #27661, None method is suggested.

    public static bool None<TSource>(this IEnumerable<TSource> source, Func<TSource, bool> predicate)
    {
      return !source.Any<TSource>(predicate);
    }

I think None method is a little similar to IsEmpty, but I don't they are same. So I do not think this is duplicate issue.

The reasons why I think so are

  • IsEmpty does not need argument that type is Func<TSource, bool> predicate.
  • None needs argument that type is Func<TSource, bool> predicate.

My suggestion method is that checks IEnumerable<T> is just empty witout arguments.

@RyotaMurohoshi
Copy link
Author

@kashifasif

Sorry, in my opinion, this method should throw NullReferenceException when enumerable is null, same as other LINQ methods.

@Gnbrkm41 Thank you!

@Gnbrkm41
Copy link
Contributor

Gnbrkm41 commented Feb 7, 2019

I believe parameter-less variant of None method could be added as well if the Predicate one were to be added, but in the issue above (#32878) (@)stephentoub mentioned that it adds little value while it has some downsides.

I understand the original reasoning, but I don't think we should be in the business of adding new APIs that are effectively just differently-named forwarders to existing APIs. They add very little value and could be done by any developer in any library with as good an implementation as the core libraries can provide. While adding little value without downsides is something to consider, there are downsides here: additional surface area a developer needs to learn, potential confusion as to when to use None vs !Any, inability to write code to a lower .NET Standard version when such a lower version could easily targeted by using the existing named API, etc.

I especially agree to the last point of not being able to write compatible library with lower versions of .NET Standard.

@RyotaMurohoshi
Copy link
Author

In my suggestion, the name of IsEmpty is so important.

In some other programming languages, there are methods that judges array, list, sequence, enumerable are empty or not. And, the names are IsEmpty or isEmpty.

I think the IsEmpty makes big value for C# beginner, especially who has experience such other programming languages. The name of IsEmpty is so clear and eplicit. And so familiar to them.


By the way, I agree this (@)stephentoub's this comment. It is so important!

But in my opinion, IsEmpty makes value.

I think everyone who participates in this issue is know how to use !enumerable.Any() and reason to use !enumerable.Any(). But I think that C# beginner is not same. For C# beginner. It is a little difficult to find usage of !enumerable.Any() to check empty for them. I think IsEmpty is so easy to find and useful to them.


How do you think about viewpoint from C# beginner? I want to listen your and others opinion.
And I think IsEmpty is more clear and more explicit.

@Gnbrkm41
Copy link
Contributor

Gnbrkm41 commented Feb 7, 2019

Summary of things so far:

Pros

  • Increases the terseness of the code, Extremely straightforward to understand and use unlike !Any() (Thus making it beginner-friendly)
  • Once added, can be used for all types that implement IEnumerable<T>

Cons

  • Essentially a renaming of !Any(), does not add a massive value
  • Would limit the portability of the code that uses this, as it is not present in older standards/frameworks (if it were to be added)
  • Requires boxing of enumerator in some cases (List.Enumerator, Dictionary.Enumerator etc...) - Isn't a problem limited to IsEmpty, common problem of all LINQ extensions but perhaps a thing to consider if it's expected to be used with List<T>, Dictionary<TKey, TValue> etc frequently

@Grauenwolf
Copy link

Grauenwolf commented Feb 8, 2019

None needs argument that type is Func<TSource, bool> predicate.

That sounds wrong to me.

We have .Any with and without a predicate. So we should have .None with and without a predicate. Having a different name for the two versions of None would be unnecessarily confusing.

@Grauenwolf
Copy link

If we are going to have an .IsEmpty then we should also have an .IsEmptyOrNull. There are definitely going to be times when a null list and an empty list should be treated the same way.

(Yes, I know this contradicts my last comment.)

@Zenexer
Copy link
Contributor

Zenexer commented Feb 8, 2019

@Gnbrkm41

I don't understand what you're saying. Do you mean IEnumerator? If that's the case, yes it is supposed to be used once. but still, you can call GetEnumerator() on IEnumerable as many times as you want to get a new one?

There's no guarantee that subsequent calls to GetEnumerator will succeed. It usually works, but that's not always the case. It may also have unintended side effects. Typically, static analyzers like ReSharper will yell at you if you reuse an IEnumerable. (If you use a more specific type like IList for which GetEnumerator can be called multiple times, you won't get the warning.)

The bigger point here, though, is that @RyotaMurohoshi makes the argument that Count() == 0 is invalid while !Any() is not. They both have the exact same risk attached in terms whether subsequent calls to GetEnumerator will succeed.

@Gnbrkm41
Copy link
Contributor

Gnbrkm41 commented Feb 8, 2019

@Zenexer Ah, I see now. Never thought of that.

@RyotaMurohoshi
Copy link
Author

RyotaMurohoshi commented Feb 8, 2019

@Zenexer OK, I see now. And I checked that how does Rider tell us this point.

2019-02-08 22 11 26

@Zenexer
Copy link
Contributor

Zenexer commented Feb 8, 2019

Just to clarify, I do agree with you, @RyotaMurohoshi: the current system isn't intuitive, and this confusion over multiple enumeration just serves to exemplify that. I don't have an easy solution, unfortunately, but it goes beyond a simple lack of IsEmpty().

This is moving a bit away from the issue at hand, but perhaps someday we'll see an attribute that can mark IEnumerables at multi-enumeration-safe. The catch there is that it's not always clear when multiple enumeration is safe: sometimes it will work but have a performance penalty, such as with Entity Framework Core. While multiple enumeration with EF Core is technically possible (and sometimes desirable), it's often more efficient to cache everything in a list.

@RyotaMurohoshi
Copy link
Author

@Grauenwolf

We have .Any with and without a predicate.

Yes. I agreen above.

So we should have .None with and without a predicate.

But sorry, I don't agree above point. And at the issue, (@)terrajobst does not talk Nome() method that has no arguments.

@RyotaMurohoshi
Copy link
Author

@Grauenwolf

IsEmptyOrNull.

It may be so valuable to talk about it. I think next points and ideas.


First.
We have the String.IsNullOrEmpty static method in BCL/FCL, so I think that IsNullOrEmpty is better than IsEmptyOrNull as the method name.

https://docs.microsoft.com/en-us/dotnet/api/system.string.isnullorempty?view=netcore-1.0


Second.
All LINQ methods throw Exception when IEnumerable<T> is null.
In C#, do you know extension methods that do not throw Exception when receiver (first argument of extension method) is null?
Maybe, it may become first one.
Someone may feel that Enumerable.IsNullOrEmpty extension method is strange, but the other one may agree it.

(Sorry I do not know it, but it may exist in BCL/FCL.)


Third.
In Kotlin: JVM Language, there is isNullOrEmpty extension methods.

  • fun Array<*>?.isNullOrEmpty(): Boolean
  • fun <T> Collection<T>?.isNullOrEmpty(): Boolean
  • fun <K, V> Map<out K, V>?.isNullOrEmpty(): Boolean

POINT!

They are extension methods for Nullable types.
They are extension methods for Array<*>?, Collection<T>? and Map<out K, V>?, not Array<*>, Collection<T> and Map<out K, V>.

https://kotlinlang.org/api/latest/jvm/stdlib/kotlin.collections/is-null-or-empty.html


Forth.

C# 8.0 will add and improve language features around null.


IsNullOrEmpty or IsEmptyOrNull may be good and valuable.

But in my opinion, I want to wait talking about it until C# 8.0 release and do not want to talk in this issue.
How about talk about it after C# 8.0 release?

@RyotaMurohoshi
Copy link
Author

Hi!

@cston
@333fred

Could you put api-suggestion label and area-System.Linq label on this issue?

@RyotaMurohoshi
Copy link
Author

@cston Thank you!

@bcronce
Copy link

bcronce commented Jun 12, 2019

Just wanted to mention that Enumerables are a form of generator. By definition they cannot be empty, but you can check if they produce/generate anything. I could see ICollection having IsEmpty added.

For correctness sake, don't conflate generators and collections. /opinion

@Zenexer
Copy link
Contributor

Zenexer commented Jun 12, 2019

@bcronce IEnumerable makes no assertions about where the data is coming from—it could be a generator, it could be a collection, or it could be something else entirely. Not only does it make no sense for IEnumerable to provide IsEmpty, it has no way of doing so because that test can’t be performed for many of the types that implement IEnumerable (particularly generators, as you mentioned).

This is rarely clear to newcomers, and I consider it a documentation/education issue. I’m not really sure how to improve the situation, but we need to find a better way of communicating the role the IEnumerable plays. It has a tendency to confuse both new programmers and experienced programmers alike. It’s an incredibly powerful tool, but it’s not particularly intuitive.

@stephentoub
Copy link
Member

stephentoub commented Jul 18, 2019

@Zenexer
Copy link
Contributor

Zenexer commented Jul 20, 2019

Edit: Never mind, I confused the OP with another commenter.

@msftgits msftgits transferred this issue from dotnet/corefx Feb 1, 2020
@msftgits msftgits added this to the 5.0 milestone Feb 1, 2020
@ghost ghost locked as resolved and limited conversation to collaborators Dec 14, 2020
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