Discosal: Null-conditional return

[lachbaer]

I just found following code snippet on https://msdn.microsoft.com/en-us/magazine/dn802602.aspx that lead me to the idea of a null-conditional (yield) return statement.

public static IEnumerable<T> GetValueTypeItems<T>(
  IList<T> collection, params int[] indexes)
  where T : struct
{
  foreach (int index in indexes)
  {
    T? item = collection?[index];
    if (item != null) yield return (T)item;
  }
}

Instead a shorter variant would be

/* ... */
  {
    yield return? (T)collection?[index];
  }

(I really dislike the heavy occurences of == null and != null)

[jnm2]

(I really dislike the heavy occurences of == null and != null)

I'd throw ArgumentNullException if the primary enumerable or collection was passed in null. Definitely would avoid incurring the overhead of a redundant null check and nullable cast on each loop iteration (collection?[index]).

public static IEnumerable<T> GetValueTypeItems<T>(
  IList<T> collection, params int[] indexes)
  where T : struct
{
  // Highly indicative of a bug; null and empty should mean two different things.
  // yield break if you absolutely must.
  if (collection == null) throw new ArgumentNullException(nameof(collection)); 

  foreach (int index in indexes)
  {
    yield return collection[index];
  }
}

[lachbaer]

@jnm2
collection[index] could also be null. And yet, please comment on the topic and not the specific example 😃

[jnm2]

@lachbaer collection[index] can't be null because T : struct, right?

[lachbaer]

@jnm2 Yes, you're right, I just copied it from the mentioned source. Should be IList<T?> then, probably.

[jnm2]

So leaving the example aside:

1. Is yield return? visible enough vs yield return?

2. I don't think var temp; if (temp != null) yield return temp; is common enough to make a pattern around it, is it? If nulls are in the source, I would be very surprised if nulls were skipped from being returned. I can't think of any use cases.

   In other words, if I say "Here is a list: 1, 2, null. Tell me what is at indexes 0 and 2," it would be very unhelpful to be told, "1." I asked for two things and was given one. Is that really "1, null" or "null, 1?"

[jnm2]

I keep reading the portmanteau in the title as a misspelling of "disposal." I'm not sure that's the sound we should be shooting for 😄

[lachbaer]

@jnm2

misspelling of "disposal."

How about "Propussion"? 😂

Like with any obj?.Do(); the whole expression is evaluated to a null; statement, even in the case that the return keyword is suffixed by a ?.

It could be a logical extension to the language to suffix other expressions with ? also, giving a consistent meaning.

On the other hand, when non-nullable reference types will come, the use cases of nulls will probably diminish anyhow.

[jnm2]

How about "Propussion?"

I knew you would go there... let's hope these threads don't lead to concussions 😁

[jnm2]

I'm strongly in favor of await? something?.Foo() and obj?.SomeEvent += handler, because I run into them _a lot_, so I could see your point about yield return?, if it was commonplace enough to warrant attention.

I'm a little concerned that the next ask would be return? for non-iterator methods, which I think goes too far, so coming from that angle I'm a bit skeptical.

[lachbaer]

@jnm2
👍 You - finally - see the point of this "Propodiscusalion" 😉

return? could be useful in case of

foreach (var item in myList)
{
    return? item.CreateObjectIfConditionMet(item);
}

or alike...

[jnm2]

return? could be useful in case of

This is why I was skeptical. ;-)

CreateObjectIfConditionMet is usually done this way:

foreach (var item in myList)
{
    if (TryCreateObjectIfConditionMet(item, out var obj)) return obj;
}

[lachbaer]

item has no TryCreateObjectIfConditionMet function 😉

You could write a local function (btw, the out var is nice now), or just use return? which in essence would be the same descriptive (at least for my eyes), but shorter.

[jjvanzon]

I had to solve a bug just a few days ago that could exactly be prevented by this construct. It took me hours to pinpoint the problem and it turned out my dear coworker did a yield return in a foreach loop before checking for null. That meant only the first item was checked and the others in the foreach loop were always ignored. Granted, with the proposed construct you could also make the mistake of not using ?, but I do feel that this is such an easy mistake to make and when you know 'yield return?' is this new cool C# feature, it could save us some hours and allows us to get on with it.

[lachbaer]

Actually, the code problems described here could be expressed in a much more meaningful way:

    return indexes.Select(index => collection[index])
                .Where(x => x != null)
                .Cast<int>();
 // or
    var result = from index in indexes
                    let item = collection[index]
                    where item != null
                    select (int)item;
    return result;

 // instead of
  foreach (int index in indexes)
  {
    T? item = collection[index];
    if (item != null) yield return (T)item;
  }

And with if? (#438)

  if? (var res = item.CreateObjectIfConditionMet(item))
    return res;

  // instead of
   return? item.CreateObjectIfConditionMet(item);

The other forms are a bit longer than return?, but I think they will lead to better understandable code.

[lachbaer]

@jnm2

I'm strongly in favor of await? something?.Foo() and obj?.SomeEvent += handler

Wouldn't it be enough for await to also accept the virtual null; statement? That is how the obj?.Foo(); is already realized nowadays (when I recall right). Then there is no need for an additional await?.

The consequences of obj?.SomeEvent += handler must be considered wisely. It would equally be that the l-value is that virtual null statement and thus just ignore the whole expression. The r-value wouldn't be even evaluated then. I would favor this, too :)

[jnm2]

@lachbaer Answers here:

• Champion "Null-conditional await" #35 (comment)
• Feature request: Allow null-propagation operator for event accessors roslyn#1276

[DavidArno]

(I really dislike the heavy occurences of == null and != null)

Me too. The approach I take is to avoid having null values. If you find yourself needing syntax sugar to help you manage a bad situation, then: stop, think and fix the bad situation...

[lachbaer]

@DavidArno

you find yourself needing syntax sugar to help you manage a bad situation, then: stop, think and fix the bad situation...

Wise saying, but that is in praxi mostly not the case. Also when being "in a flow" most people probably don't "stop", but keep on writing. After a while there already is working code, maybe actually not even bad, but nevertheless you end up with all those =! nulls.

Besides, I'm currently preparing a discussion with a more modern C#-like syntax. Here a "prototype"

static int Foo(object obj)
{
  return when (obj == null) with (default)
    Debugger.Log("Calling Foo with 'null' object?");
}

[DavidArno]

@lachbaer,

After a while there already is working code...

Great, so you have reached the "green" stage of "red, green, refactor". So the next step is: refactor. Rewrite that code, getting rid of all those != null's. The tests still pass, so all is well. Then on to writing that next test, and back to "red" once more...

[lachbaer]

@DavidArno

So the next step is: refactor

I don't have enough time for that! True! Period!

Code I write is done when it works, then the next issue must be solved. And like me there are thousands of thousands of programmers that do not work in a bigger team...

@DavidArno : why 👎 ? It's a fact!

[lachbaer]

I now rather go with my proposal #453 "Conditional branches, return when (...) with (...) " that is nearly equally short but more expressive.

[DavidArno]

@lachbaer,

Because it's not a fact. Unless you are writing throw-away code that you'll never revisit, your "I don't have time" attitude means you are simply choosing to incur technical debt by not improving your code as you write it. That technical debt then will never be paid off as you will forever be fighting with bad code you wrote before and so will have even less time to make good the new code. And you will never escape that downward spiral until you stop saying "I don't have enough time for that!" and start saying "I will make time for that". Thus the 👎

[jnm2]

@DavidArno is saying what I was thinking but didn't say.

[lachbaer]

@jnm2 @DavidArno
I kindly ask whether you can remove your 👎 's from my comment above. I feel like it makes me - as the auther - being stubborn, but I am definitely not!

Maybe you are coding in a bigger team. I studied economic computer science, too, and from the theoretical point or practical point in team projects I am absolutely with you!

But as a freelancer currently I sometimes write some tools here and there for customers and others, besides other work. When they are done, I simply do not have more time to improve the coding, that time wouldn't get paid anyhow. Please respect that. Thanks!

[jnm2]

@lachbaer I respectfully disagree with you when it comes to the practical, real world. If you don't have time to refactor as you go, you end up missing crucial implementation issues. If the project gets beyond a very small size, you start to have architecture issues. These things cost more money than you save by avoiding refactoring. I speak as a primarily lone programmer. Believe me, I've been in situations where I just wanted to ship and felt like I had no time to clean up and design better. I learned the hard way.

[lachbaer]

@jnm2 I do refactor a lot. But not to the degree that is mentioned here sometimes. I even started some projects with "a bigger picture" in mind, just to end up starting over with a more "streamlined" version, meaning lesser classes and dependencies - just to get finished. The "bigger picture" were certainly the better, but it simply took too long to finish.

[lachbaer]

@jnm2

Believe me, I've been in situations where I just wanted to ship and felt like I had no time to clean up and design better. I learned the hard way.

Me, too. But also the other way round 😉 Guess, I've seen both sides of the coin. 😜

[jnm2]

@lachbaer More streamlined is typically better than guessing at a bigger picture when you only have guesses. I don't think that kind of thing is what people are talking about in this thread. The discussion is about not liking to see == null and != null even though you've ended up going for that design. I say having to see it all over the place is more transparent and truthful to reality.

[lachbaer]

I say having to see it all over the place is more transparent and truthful to reality.

Agreed. But it is SO ugly! At least != null.

Meanwhile I even like == null - ups... 😄 I have inspected some code and came to the conclusion that in the used scenarios anything other than obj == null is not descriptive enough.

There is only one exception, being if (obj == null) return;-constructs. That's why I came up with this and #453.

[HaloFour]

dotnet/roslyn#4323 (comment)

[lachbaer]

@HaloFour Thanks for the link! 😃 As I previously said, I don't feature this "Discosal" any more.

[gafter]

Closing, as this proposal is no longer supported by its author.

[ghost]

@deaddog @Pilchie @zippec @gafter @HaloFour
I was about to ask for the same thing, while I was writing this code:

var t = CheckToolStrip(frm.ContextMenuStrip);
if (t != null)
   return t;

t = CheckToolStrip(frm.MainMenuStrip);
if (t != null)
    return t;

Which will become:

return? CheckToolStrip(frm.ContextMenuStrip);
return? CheckToolStrip(frm.MainMenuStrip);

I even want to make it:
return? CheckToolStrip(frm.ContextMenuStrip) : CheckToolStrip(frm.MainMenuStrip);
Where we can add more :s and return values. The first non-null value will be returned.
Hope you reconseder this proposal. It is very useful.

[CyrusNajmabadi]

You can already do this today:

return CheckToolStrip(frm.ContextMenuStrip) ?? CheckToolStrip(frm.MainMenuStrip);

[CyrusNajmabadi]

Hope you reconseder this proposal. It is very useful.

@MohammadHamdyGhanem This issue has been closed. If you have a fleshed out proposal, i would recommend creating a new issue.

[ghost]

@CyrusNajmabadi
return CheckToolStrip(frm.ContextMenuStrip) ?? CheckToolStrip(frm.MainMenuStrip);
is different than:
return? CheckToolStrip(frm.ContextMenuStrip) : CheckToolStrip(frm.MainMenuStrip);
which will not return if CheckToolStrip(frm.MainMenuStrip) is also null, allowing any following code to be executed.

[ghost]

But ?? helps in my use case:

        ToolStripItem FindTSI(Form frm) =>        
            CheckToolStrip(frm.ContextMenuStrip) ?? 
                CheckToolStrip(frm.MainMenuStrip) ?? 
                CheckControls(frm);

I'm not used to use the ?? operator, so I nearly forgot it!
Thanks.

On other hand, one can use many ? in one line, which maked code shorter to write but hard to read:
Console.WriteLine(s ?? FindTSI(this)?.ToString());
Maybe '?' and '??' should be colored (say in orange)!

[ghost]

For example, I can't use ?? in this function:

        ToolStripItem CheckControls(Control parent)
        {            
            foreach (Control c in parent.Controls)
            {
                if (c.Visible)
                {
                    var t = CheckToolStrip(c as ToolStrip);
                    if (t != null)
                        return t;

                    t = CheckControls(c);
                    if (t != null)
                        return t;
                }
            }

            return null;
        }

because it will break the loop after first iteration even both functions return null. The following code is not equivalent to the previous one:

        ToolStripItem CheckControls(Control parent)
        {            
            foreach (Control c in parent.Controls)
            {
                if (c.Visible)
                {
                   return CheckToolStrip(c as ToolStrip) ?? CheckControls(c);
                }
            }

            return null;
        }

But this would be:

        ToolStripItem CheckControls(Control parent)
        {            
            foreach (Control c in parent.Controls)
            {
                if (c.Visible)
                {
                   return? CheckToolStrip(c as ToolStrip) : CheckControls(c);
                }
            }

            return null;
        }

[CyrusNajmabadi]

@MohammadHamdyGhanem This issue has been closed. If you have a fleshed out proposal, i would recommend creating a new issue.