dotnet / csharplang

The official repo for the design of the C# programming language
11.53k stars 1.03k forks source link

Proposal: Null-Coalescing foreach #3034

Closed SkyeMacMaster closed 4 years ago

SkyeMacMaster commented 4 years ago

I want to propose a new null-coalescing feature.

Currently, if you have a enumerable variable, you have to nest the loop to check if it's null.

if(list != null) { foreach(var item in list){ } }

My proposal is to use ? to simplify this and reduce nesting.

foreach(var item in list?){ }

Thanks, Skye

ronnygunawan commented 4 years ago

2727

1081

theunrepentantgeek commented 4 years ago

There's also this existing approach to consider:

foreach (var item in list ?? Array.Empty<Item>())
{
}

This achieves the stated goal of reducing nesting.

That said, this neither is a code pattern I see at all regularly in code - a null list (or collection or sequence) is almost always a code smell indicating a programming error elsewhere.

akraines commented 4 years ago

Or something like this:

//Extension method
public static IEnumerable<T> NullToEmpty<T>(this IEnumerable<T> enumerable)
{
   return enumerable ?? Enumerable.Empty<T>();
}

//Usage:
foreach (var item in list.NullToEmpty())
{
}

(EDIT: I just saw that @HaloFour suggested the same approach here)

TonyValenti commented 4 years ago

Here are my thoughts on this:

  1. This seems like a feature that pretty much everybody asks for.
  2. All of the extension methods proposed have one weakness: Parent?.Child?.Collection?.NullToEmpty() doesn't work when the path contains null items.

I would love a `foreach?(....) or the priority of `?. to change so that this common request is available in a friendly and easy to use way.

ronnygunawan commented 4 years ago

Is there any reason why nobody has considered using ?.ForEach for this case?

list?.ForEach(item => {

});
Joe4evr commented 4 years ago

@ronnygunawan Because 1) that method only exists on List<T> and 2) it's well-established that that method is kinda terrible.

ronnygunawan commented 4 years ago

2) it's well-established that that method is kinda terrible.

It was posted before C# 6.0 and null conditional, though. The argument may no longer be valid.

svick commented 4 years ago

@TonyValenti

  1. All of the extension methods proposed have one weakness: Parent?.Child?.Collection?.NullToEmpty() doesn't work when the path contains null items.

(Parent?.Child?.Collection).NullToEmpty() should work.

333fred commented 4 years ago

It was posted before C# 6.0 and null conditional, though. The argument may no longer be valid.

The core point of Eric's article is that the Linq apis are entirely about methods that do not cause side effects, and a ForEach method is only about causing side effects.

WrongBit commented 4 years ago

I support this proposal even w/o '?' mark. Because foreach is a high level, highly used construct and should not waste attention on all checks. If list is empty or null, it's not an error, it's "no iteration" situation. And only if business logic requires, programmer can check list variable to be non-null.

HaloFour commented 4 years ago

@WrongBit

If list is empty or null, it's not an error, it's "no iteration" situation.

null is not empty, and that would be a breaking change affecting how the language has worked since 1.0.

HaloFour commented 4 years ago

@333fred

The core point of Eric's article is that the Linq apis are entirely about methods that do not cause side effects, and a ForEach method is only about causing side effects.

That's an interesting perspective. I disagree slightly, tho. ForEach isn't about side-effects. Side-effects are secondary to the primary effect, and the iteration within ForEach is the primary effect as it is a terminal operation. A side-effect would be doing something like modifying state within Select or Where where it's not expected. Side-effects are certainly undesirable in functional programming, but there's nothing wrong with ForEach. In fact that method is the idiomatic way to iterate over collection-likes in Scala rather than using a for loop.

WrongBit commented 4 years ago

@HaloFour : It could be breaking change if NRE were caught at compile time. But it's RUNTIME error, which do NOT correspond to the language. So you're wrong, we can quite easy introduce "nullable foreach" - all existing code already check collection for null, while new code will get benefit of relaxed rules. You cannot protect BAD DESIGN just hiding behind "compatibility". C# inherited many weak points from Java (being its copy), so it's time to confirm mistakes and... fix it!

HaloFour commented 4 years ago

@WrongBit

It changes the semantics of the language. That is, by definition, a breaking change. Whether or not you agree with the current design is completely irrelevant.

theunrepentantgeek commented 4 years ago

@WrongBit

This is a breaking change, in this way:

Code that currently throws, and catches, an NRE when the list is null, taking a different execution path.

This exact same concern is why the LDM didn’t decide to use negative integer indexes when indexing into arrays to obtain trailing items - people can, and do, catch the out-of-range exception.

333fred commented 4 years ago

That's an interesting perspective. I disagree slightly, tho. ForEach isn't about side-effects. Side-effects are secondary to the primary effect, and the iteration within ForEach is the primary effect as it is a terminal operation. A side-effect would be doing something like modifying state within Select or Where where it's not expected. Side-effects are certainly undesirable in functional programming, but there's nothing wrong with ForEach. In fact that method is the idiomatic way to iterate over collection-likes in Scala rather than using a for loop.

@HaloFour from the perspective of the ForEach extension, the only output that can possibly be observed is something captured by the lambda and modified inside. That's why the only bolded line in Eric's article is "Clearly the sole purpose of a call to this method is to cause side effects." Whether you believe that these should be considered side effects is a separate question, I guess, but personally I would, and wouldn't support a ForEach extension for that reason.

SkyeMacMaster commented 4 years ago

@HaloFour I don't see how this could be a semantic change (IE breaking change). The following two examples seem semantically the same to me.

foreach (var item in list.NullToEmpty()) { }

foreach (var item in list?) { }

People who want to treat a null list as an empty list are still just as free to do so as people who want a runtime exception when trying to use a null list.

HaloFour commented 4 years ago

@SkyeMacMaster

It wouldn't be a breaking change if it involved syntax changes. The suggestion was to have foreach simply ignore null enumerables, which would be a breaking change.

SkyeMacMaster commented 4 years ago

@HaloFour Oh, overlooked that comment. I certainly wouldn't suggest doing that. That would be a breaking change.

thargol1 commented 4 years ago

As @theunrepentantgeek already mentioned having null-list is a code smell. This problem should be fixed at the source. So change the method that returns the null-list to return an empty list. Or if that's not possible at the ??-operator or the .NullToEmpty() after the method call.

If you're worried about allocating an empty list, maybe you can use Array.Empty<T>, I believe that one uses no extra memory.

WrongBit commented 4 years ago

I disagree to name it "breaking change". If your code was so clumsy that only in foreach you discover "oh, my list is null!", throw away your code. It's smelly piece of "student work". You have to check a list for null when list was first time obtained. ESPECIALLY when current foreach implementation disallow nulls! In other words, ALL CURRENT CODE, written by normal people already has check for null. And all I offer is to get rid of unnecessary restriction.

Many times in .NET API I meet situation when developers like dictators prohibit ANY "wrong" behaviour! It's too stupid to think "the world should dance by my rules". There is thousands situations when task has "not so good" data, but that data has no big matter in terms of algorithm.

Example: In Perl if you get non-existing element of Dictionary, you... simply get it! (it's null obviously) But not for C# smarties! They throw exception with and without reason. So if you check (as example) command-line args like:

if (arguments["ShowHelp"] == "yes") ....

...then you just get by rack in forehead! Hell, WHY?? WHAT is wrong if "ShowHelp" key do not exist?

I describe this analogy to say how funny same restriction works for foreach. If we REALLY WORRY about 'nullable' list - just check it! It's YOUR code, what's a problem?? But if you don't care about content of the list, why you should care it's not null at all? Thousands APIs returns hell knows what - some returns null, some empty list, some throw exception, some returns error code... We simply cannot make 'em "one style". Meaning I must not care about things like:

foreach(var f in StupidFuncReturnsDirectoryListOrNull()) { }

I do not care at all, is it null, is it empty list or whatever - I just "jump over" this iteration and viola! But you force me to write like this:

var lst = StupidFuncReturnsDirectoryListOrNull();
if (lst != null) {
    foreach(var f in ) { }
}

It's one more nested operator and 3(!) additional lines I don't care about.

When you ask people about time, how you do it?

John, what time is it?

But "microsoft way":

John, do you have watches? If you do, please look at 'em and tell me what time it is.

Is anybody so boring to ask first DO YOU HAVE WATCHES?? No watches - it's null nobody care about. Simple as life. And complex like MS logic. :))

HaloFour commented 4 years ago

@WrongBit

John, what time is it?

When John doesn't have a watch and tells you, "I don't know, sorry," that is the equivalent of a NullReferenceException.

WrongBit commented 4 years ago

@HaloFour Nope. Behaviour of John in terms of C#: "Hey, WTF you ask me? Open your eyes, do you see any watches on my arm?! Let me kick your ass, until you fall on pieces, because you did mistake - you ask me about time when I had no watches!". :) THIS is "null ref exception"!

HaloFour commented 4 years ago

@WrongBit

No, no it's not. And your projection isn't useful for the conversation. You're asking for John to ignore you if he can't furnish the answer, as if you're supposed to move on. But either way, you're asking for the behavior to change from what it has been for the past 20 years. That is a breaking change, and it frankly doesn't matter in the slightest how much you dislike the current behavior.

WrongBit commented 4 years ago

In other words, "nothing will be changed, because we follow wrong way 20 years and we're too lazy to fix it". I understand you, but not accept your way. You're like husband who don't like his wife for 20 years, but too scared to divorce. Yes, way to ideal language cannot be easy if you start from WRONG THINGS (like Java, he-he). Developers of the D language wasn't that "chicken" and made a lot of breaking changes before they make language perfect. Exemplary I would say. (am I right that "ranges" you steal from there? :) )

WrongBit commented 4 years ago

"You're asking for John to ignore you if he can't furnish the answer" - EXACTLY! I can easy pass John w/o getting answer, because it's not important. Cycle is not "must execute" code - it's just one of many steps in algorithm. No cycle - OK, execute next instruction! What's wrong there?

HaloFour commented 4 years ago

@WrongBit

Nothing will be changed because it's not broken. You not liking the current behavior does not make it broken. There's nothing else to talk about.

333fred commented 4 years ago

am I right that "ranges" you steal from there? :)

Many languages have range types, in quite a few different ways. The wonderful thing about this repo is that you can go back and see the meeting notes from previous years and see how we arrived at the current design.

In other words, "nothing will be changed, because we follow wrong way 20 years and we're too lazy to fix it".

No, we just don't take breaking changes to C# unless there's a really, really compelling reason. And this particular behavior is so old that we'd just never take the break, period. I guarantee you that someone's code depends on this behavior, and regardless of whether we argue that that's good or bad practice, that's the reality of development.

This discussion is getting off the original proposal. We are not changing the semantics of existing foreach with regards to null, full stop. Let's keep the discussion off that, and on the original topic.

AartBluestoke commented 4 years ago

I think there is a disagreement about what null means;

asking "foreach(null)" is like asking "how many cars in my garage?" (my apartment has no garage, or any equivalent place that i can put a car) -- it's the question that doesn't make sense, not the answer. foreach(empty) is "how may car parks slots do i have in the nearby carpark" (that's the empty list ...)

i believe the current language design within c# is that attempts to use NULL in c# means "your attempt to use this variable was silly", and provides Enumerable.Empty() is the efficient way to represent the list of no objects. This is reinforced by the recent introduction of null reference tracking.

I believe the language philosophy suggests that nulls should be detected and removed as early as possible, preferably wherever you got the nullable list from should instead be fixed, or you should deliberately say which actual thing you want to operate on
foreach (var z in x??Enumerable.Empty<thing>()) {};

or even further up in your code

var x = f(y)??Enumerable.Empty<thing>());
foreach (var z in x){...}

foreach is equivalent to getting an Enumerator and asking for the next. I don't think the language should state that there is a specific answer to "does the null Enumerator have a next value"?

WrongBit commented 4 years ago

@AartBluestoke : Taking one of edge positions is always loose.

I believe the language philosophy suggests that nulls should be detected and removed as early as possible

Not everywhere! It's stupid to think "null is evil" (as well as 'goto'). I already show example (which you obviously ignored). Say, we wanna process files in some folder:

foreach(var f in StupidFuncReturnsDirectoryListOrNull()) { }

In this case we don't care at all is it null, empty list or "pi" number. And it's not narrow case - null-s come everywhere and ONLY when business logic requires, we have to check for null.

Fighting against null is same stupid as fight against 'goto' - (unprofessional) people just cannot handle it properly. But who said such people should write serious programs?? Language is made for professionals. THEY know when and what to use. If I need null - I will use null and my program will work absolutely smooth. If you wanna make safe language for dumb people, just give 'em "turtle language" and keep 'em away from designing C#! THIS is core of problem, not poor 'null' entity.

thargol1 commented 4 years ago

@WrongBit I can see your point, but look it at another way. Suppose you have a function returning a directory list. The function returns:

  1. null when the directory does not exist.
  2. Empty list when the directory does exist but contains no files.
  3. A filled list when the directory dies exist and contains files.

This is a valid use of null.

If you want to use a null coalescing foreach on the result of the function then either option 1 of option 2 are not used and in my opinion the function should be altered to return an empty list when the directory does not exists.

You want to fix the problem where the function is consumed. I think the problem should be fixed where the return value is created. And looking at the other comments, I don't think you will get what you ask for.

WrongBit commented 4 years ago

@thargol1 : Sorry, didn't see anything like "another way look". I see at foreach as a enumeration operator. And it's absolutely legal situation when collection has no items (or collection is null). And only when your business case requires, you still have ability to get list in a separate variable and check it for null. Currently C# "offends" people who don't want to check for null, but they have to.

I repeat: programming is NOT for mediocres who "likes to program". If you're careless teenager who wanna program, but don't want to worry about correctness - program on "turtle language". C# is for professionals who makes serious programs. C# has no space for crutches, which care about your negligent code. THIS is my main concern: dirtying C# in favor of indian "super developers" after 2-weeks course "C# for dummies".

YairHalberstadt commented 4 years ago

@WrongBit Please avoid making any comments on here that appear to charecterize developers of a specific country negatively. It may be that you didn't mean it offensively, but there is absolutely no need for it here, and it can only cause offence.

WrongBit commented 4 years ago

@YairHalberstadt : While I agree that this specific "foreach" has no deal with countries, my heart is bleeding when I see how IT is occupied with thousands absolutely incompetent monkeys. And point is they not only sit in PC, but DEMAND "let's make C# more safe, because I'm so incompetent to write proper code". NOW do you see where is the link? That's why I'm too skeptical when I read next news "MS improved C#". They didn't. They just f$$-up the language and most voters for this sh***t coming from.....? Yep, you know the answer.

CyrusNajmabadi commented 4 years ago

let's make C# more safe

and most vote[s] ... coming from?

The desire for making language safer have been driven hugely from the needs of MS themselves. Every bit of unsafety leads to massive problems that cause a very real monetary cost for the company. Safety is desirable, and sought by so many (including MS and many core partners) precisely because it produces results that perform better on so many metrics.

I would highly recommend stepping back and evaluating how you communicate here. Your behavior will win you no favors or supporters, and you've already egregiously crossed the line in terms of acceptable behavior. If you want to continue posting here, you're going to need to be able to behave in a more professional manner.

For more information please see the .Net CoC: https://dotnetfoundation.org/code-of-conduct

terrajobst commented 4 years ago

@WrongBit your comments have crossed the lines several times (and not just in this thread). I’ve blocked you for 7 days. If you keep doing this, I’ll block you permanently. Disagreeing with our design choices is fine, quasi racist behavior is not.