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 IsEmpty that judges that sequence is empty or not #561

Closed

Conversation

RyotaMurohoshi
Copy link

Summary

Add IsEmpty method.

  • Judges that IEnumerable<T> is empty.
  • Someone uses "Count() == 0" incorrectly. IsEmpty method helps them.
  • This is so declarative and easy to find from Intellisense.

Background

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

I suggest IsEmpty method to MoreLinq. 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.

In other languages.

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

Discussion

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.

@leandromoh
Copy link
Collaborator

You can use "AtMost(0)" or "Exactly (0)" for clarity

@RyotaMurohoshi
Copy link
Author

I think that IsEmpty is more clearer than AtMost(0) and Exactly(0).

Though there are AtMost and Exactly in MoreLinq, I think IsEmpty is so good for MoreLinq.


  1. More straight and declarative.

The method name IsEmpty is more straight and declarative to check the sequence is empty or not.

Someone understand all of MoreLinq method, but the other one understand only some methods in MoreLinq.
I think that someone cannot understand AtMost and Exactly method meanings from only their method name.
IsEmpty can tell is meaning and feature with its name.


  1. IsEmpty in other programing langauage.

IsEmpty is so useful to C# beginner who has other programing language experience.

Please see #561 (comment)

Copy link
Member

@atifaziz atifaziz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In general, we try to avoid trivial one-line implementations in MoreLINQ for the sole purpose of increased readability. They are best left as local extension methods in a project, and this one seems to fit that bill:

public static bool IsEmpty<TSource>(this IEnumerable<TSource> source) =>
    !source.Any();

See #556 for some discussion and thoughts on “why” for another case and I'll repeat part of the closing comment:

Also something that might not be obvious is the maintainer's dilemma: the trouble of allowing one-liners for purpose of readability means that many such requests will start pouring in and I fear that discussions will end up getting very subjective. The primary objectivity I'd like to maintain is therefore that a suggestion must be operationally intelligent or do some amount of non-trivial work. And even with that, we've struggled with the naming of many operators in MoreLINQ because one name might speak more to some people than another.

@atifaziz
Copy link
Member

atifaziz commented Feb 23, 2019

Never fun to not merge and I hope this doesn't discourage future contributions. I would recommend opening an issue to discuss before going too far with efforts to implement, test and document.

@atifaziz atifaziz closed this Feb 23, 2019
@RyotaMurohoshi
Copy link
Author

Never fun to not merge and I hope this doesn't discourage future contributions. I would recommend opening an issue to discuss before going too far with efforts to implement, test and document.

I'm sorry to open PR without issue. Next time, I will open issue before PR. Thank you.

@RyotaMurohoshi
Copy link
Author

RyotaMurohoshi commented Feb 23, 2019

I have a question.

According to your next comment,

The primary objectivity I'd like to maintain is therefore that a suggestion must be operationally intelligent or do some amount of non-trivial work.

In MoreLinq, will one-liner method be never accepted?

According README.md

LINQ to Objects is missing a few desirable features.
This project enhances LINQ to Objects with extra methods, in a manner which keeps to the spirit of LINQ.

I belive that IsEmpty and ConcatAll provides big readabilty and they are desirable features.


By the way I think that principale, core rule and core concepts are so important for Library.

The primary objectivity I'd like to maintain is therefore that a suggestion must be operationally intelligent or do some amount of non-trivial work.

I do not agree it, but I understand it.
It is so nice that you maintain MoreLinq with principle, core rule and core concepts.


In MoreLinq, will one-liner method be never accepted?

@atifaziz
Copy link
Member

atifaziz commented Feb 23, 2019

LINQ to Objects is missing a few desirable features.
This project enhances LINQ to Objects with extra methods, in a manner which keeps to the spirit of LINQ.

I belive that IsEmpty and ConcatAll provides big readabilty and they are desirable features.

That's a very old motto (from @jskeet) that's been there since the inception of the project but I do think it's still applicable. From discussions, I have found that some people believe that the spirit of LINQ is to make code more readable. While that's true, it's also a very shallow observation.

I do not agree it, but I understand it.

I respect you for holding your view with grace. 👍

In MoreLinq, will one-liner method be never accepted?

Most probably but I won't say never.

I belive that IsEmpty and ConcatAll provides big readabilty and they are desirable features.

Like your LinqAlias project, I think there's a home for such “one-liners for sake of improving readability” but which don't necessarily add operational intelligence. If you start such a project, I'd be happy to direct people there.

@RyotaMurohoshi
Copy link
Author

Thank you for your answer. OK. I understand!

By the way, my aim was not add only 'one liner methods'.
I want to use useful collection methods that is in many other programming language but is not in C# LINQ to Objects.

For example,

Many of them are in MoreLINQ. So I am happy if IsEmpty and Flatten(ConcatAll) are added in MoreLinq. Certainly IsEmpty and Flatten(ConcatAll) are 'one liner methods', but I believe that they are so meaningfula and useful.


I think MoreLINQ is so nice library! And I want to send idea or issue in the future.

By the way, I will start project that provides useful collection methods that is in many other programming language but is not in C# LINQ to Objects.

Thank you for your answer.

@atifaziz
Copy link
Member

atifaziz commented Mar 7, 2019

And I want to send idea or issue in the future.

I look forward to that. 👍

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

Successfully merging this pull request may close these issues.

3 participants