Proposal: add else block to foreach if no items are iterated

[jcouv]

@EBrown8534 commented on Sat Jan 28 2017

Consider the following ASP.NET Razor sample:

<div class="content">
    @foreach (var item in Model
        .Items
        .Where(x => x.Name != null)
        .OrderByDescending(x => x.Added))
    {
        <div class="item">
            @item.Name
        </div>
    }
</div>

If one wishes to check that the collection would first return items it requires adding another level of testing to the Razor:

<div class="content">
    @if (Model.Items.Count(x => x.Name != null) > 0)
    {
        foreach (var item in Model
            .Items
            .Where(x => x.Name != null)
            .OrderByDescending(x => x.Added))
        {
            <div class="item">
                @item.Name
            </div>
        }
    }
    else // We didn't actually find anything
    {
        <div class="none">
            No items
        </div>
    }
</div>

You could propose two major solutions currently supported by the language to solve this issue:

First: create an intermediary variable in the view:

<div class="content">
    @{
        var filteredCollection = Model.Items.Where(x => x.Name != null);
        if (filteredCollection.Count() > 0)
        {
            foreach (var item in filteredCollection.OrderByDescending(x => x.Added))
            {
                <div class="item">
                    @item.Name
                </div>
            }
        }
        else // We didn't actually find anything
        {
            <div class="none">
                No items
            </div>
        }
    }
</div>

Obviously this is not a pure solution: our view contains a local variable and requires a local variable if using this solution for each and every location we wish to operate on a collection in this manner with.

The second solution would be to add the filteredCollection to the model being passed to the view, then simply:

<div class="content">
    @if (Model.ItemsWithName.Count() > 0)
    {
        foreach (var item in Model.ItemWithName.OrderByDescending(x => x.Added))
        {
            <div class="item">
                @item.Name
            </div>
        }
    }
    else // We didn't actually find anything
    {
        <div class="none">
            No items
        </div>
    }
</div>

This increases maintenance with the model, and requires this to be somehow derived from the model. (If it's a simple filter on another collection in the model that's easy enough, but the more complex our conditions become the more maintenance on the model is required.)

---

I propose we add an else block to be added directly after a foreach block which would only execute if no items were iterated.

<div class="content">
    @foreach (var item in Model
        .Items
        .Where(x => x.Name != null)
        .OrderByDescending(x => x.Added))
    {
        <div class="item">
            @item.Name
        </div>
    }
    else // We didn't actually iterate anything
    {
        <div class="none">
            No items
        </div>
    }
</div>

This would be most helpful in ASP.NET applications where custom object sorting and filtering is more likely to be common and the potential to display a 'no items to display' message is much higher.

---

@jnm2 commented on Sat Jan 28 2017

I've had the same wish, and in the same circumstances. I wonder if it could just be Razor sugar or if it would really be helpful in C# itself.
Razor could insert a local bool which gets set each time the loop iterates, and generate an if (!iterated) block instead of the else keyword.

---

@HaloFour commented on Sat Jan 28 2017

Dupe of #134.

---

@HaloFour commented on Sat Jan 28 2017

Even if reconsidered I think that there are ambiguity concerns to consider. The following is already legal syntax:

if (condition)
    foreach (var item in array) {
        // do something
    }
    else {
        // do something else
    }

---

@jnm2 commented on Sat Jan 28 2017

Can you link the actual dupe?

---

@HaloFour commented on Sat Jan 28 2017

@jnm2 Oops, typo. #134 (I'll fix the original comment too)

---

@jnm2 commented on Sat Jan 28 2017

@HaloFour This isn't a dupe if it's implemented as Razor sugar

---

@eyalsk commented on Sat Jan 28 2017

@jnm2 Then why it needs to be implemented at the language? they can add a special syntax and deal with it themselves.

---

@jnm2 commented on Sat Jan 28 2017

@eyalsk

Then why it needs to be implemented at the language? they can add a special syntax and deal with it themselves.

Correct, it would go in a different repo.

---

@alrz commented on Sat Jan 28 2017

There's no such thing as "razor sugar". Razor passes code sections as-is to the generated view class.

---

@jnm2 commented on Sat Jan 28 2017

@alrz Razor is sugar 😆
This doesn't seem that complex to add to the parser that separates C# and HTML.

---

@initram commented on Mon Jan 30 2017

For a lot of cases you can use the extension method DefaultIfEmpty(), where you can provide an element to return if the IEnumerable is empty.

---

@Thaina commented on Mon Jan 30 2017

We should just let else + any keywords format in the same line. And you should swap logic for empty on top

if(array.IsNullOrEmpty())
{
    // default logic
}
else foreach(var obj in array)
{
    // iter logic
}

It already valid syntax (need to add IsNullOrEmpty extention method) just formatter make it annoying separate line

---

@Thaina commented on Mon Jan 30 2017

On a second thought. if no one mind this syntax just it conflict with valid syntax we can just tweak any syntax

foreach if(var foo in bar)
{
    Console.WriteLine(foo);
}
else Console.WriteLine("no foos in the bar");

// or

if foreach(var foo in bar)
{
    Console.WriteLine(foo);
}
else Console.WriteLine("no foos in the bar");

// or for

for(var foo in bar) // introduce in to work in for in addition to ;;
{
    Console.WriteLine(foo);
}
else Console.WriteLine("no foos in the bar");

// or colon

for(var foo : bar)
{
    Console.WriteLine(foo);
}
else Console.WriteLine("no foos in the bar");

---

@TonyValenti commented on Tue Jan 31 2017

Wow! What an elegant solution! I love this proposal. I've always just resorted to doing something like having an extra count variable and then IFing off it. I really like this suggestion a lot.

---

@alrz commented on Tue Jan 31 2017

If this is common in Razor, you could use a partial view or even an action result filter. If there is a use case outside ASP.NET, that could be also addressed via similar mechanisms, depending on problem domain.

---

@jcouv commented on Sat Oct 21 2017

I'll move this to csharplang (where language design is now discussed). Thanks

[jnm2]

This would play off if statements and compound the dangling else problem. Nesting if inside loops and vice versa is terribly common.

[MkazemAkhgary]

what's the problem with following?

bool flag = false
foreach(...)
{
    flag = true;
}
if(!flag) // we didn't iterate anything

[MkazemAkhgary]

also Count forces iteration of entire collection, alternatively you can use Any from linq which only iterates first item in collection and returns true if any item exist.

[EBrown8534]

@MkazemAkhgary That feels like a workaround more than a natural attempt, if we were going that route it would be better to make use of existing syntax with LINQ and work it opposite:

@if (collection.Any())
{
    foreach (var item in collection)
    {
    }
}
else
{
    // No items iterated
}

Yes, it's more syntax, but at least it preserves the current scope, and doesn't open us up to additional issues via the use of new variables. (Especially when you might have several of these on a page, then the second bool flag = false; is simply flag = false; and leaves us open to nasty bugs.)

It should be completely natural within the scope of the language to do something specifically if no items were iterated, that way we don't introduce more error potential.

[EBrown8534]

@jnm2 I agree, in a situation where you're omitting braces:

if (someReasonToIterate)
    foreach (var item in collection)
    {
    }
else
    // Which else do we do here? No items to iterate, or `!someReasonToIterate`?

However, if you include braces, the syntax is no longer ambiguous:

if (someReasonToIterate)
{
    foreach (var item in collection)
    {
    }
}
else
    // Perfectly clear that we're talking `!someReasonToIterate`

And:

if (someReasonToIterate)
{
    foreach (var item in collection)
    {
    }
    else
        // Perfectly clear that we're talking no items iterated
}

[tuespetre]

@Thaina:

And you should swap logic for empty on top

Problem with that is IEnumerables -- checking for Any() or Count() or something like that could introduce unnecessary overhead depending on the IEnumerable implementation. I think your next idea is the best:

if foreach(var foo in bar)
{
    // Do stuff
}
else
{
    // Do stuff
}

[MkazemAkhgary]

checking for Any() or Count() or something like that could introduce unnecessary overhead depending on the IEnumerable implementation

define overhead, Count from linq will cause duplicate enumeration. But Any does not. Most collections have Count property so you only check length. If that still is problem, which is very rare case, then you can use flag, as i suggested above.

You are willing a feature for very rare case, it convoultes current c# syntax with very minor improvement. I dont see benefit of this syntax over existing solutions.

also foreach is not if statement. They have completely different semantics. Lets not mix things together.

[tuespetre]

@MkazemAkhgary:

define overhead, Count from linq will cause duplicate enumeration. But Any does not.

Any may not cause complete enumeration, but the partial enumeration that occurs may incur some overhead -- whether that be reading from a file, a database, an Active Directory query, etc.

You are willing a feature for very rare case

It's not very rare, especially in UI-land.

then you can use flag, as i suggested above.

The if foreach syntax would probably be a sugar for the flag approach.

[whoisj]

Since you have to assume that IEnumerable cannot be reset or enumerated more than once, any use of .Count() or .Any() corrupts the entire set. This leaves either the foreach ( ... in .. ) { ... } else { ... } model suggested manually manipulating the IEnumerator.

    public static class IEnumerableExtensions
    {
        public static void ForEach<T>(this IEnumerable<T> enumerable, Action<T> perItem, Action noItems)
        {
            var enumerator = enumerable.GetEnumerator();

            if (enumerator.MoveNext())
            {
                var current = enumerator.Current;

                perItem(current);

                while (enumerator.MoveNext())
                {
                    current = enumerator.Current;

                    perItem(current);
                }
            }
            else
            {
                noItems();
            }
        }
    }

[AartBluestoke]

Could ForEach return Boolean to indicate if it managed to iterate any objects? This would enable if(!ForEach(....)){} syntax with nothing else changed.

    public static class IEnumerableExtensions
     {
        public static Boolean ForEach<T>(this IEnumerable<T> enumerable, Action<T> perItem)
        {
            var enumerator = enumerable.GetEnumerator();

            if (enumerator.MoveNext())
            {
                var current = enumerator.Current;

                perItem(current);

                while (enumerator.MoveNext())
                {
                    current = enumerator.Current;

                    perItem(current);
                }
                             return true;
            }
            else
            {
                return false;
            }
        }
    }

[MkazemAkhgary]

@AartBluestoke cool, if thats breaking change, im not sure if it is, they can add method TryForEach

That would also handle NRE

[AartBluestoke]

I don't know if it a breaking change, would someone with more knowledge be able to comment on the feasibility of something like that?

[HaloFour]

@AartBluestoke

There is no ForEach extension method in the BCL for IEnumerable<T>, so there's nothing to break. However they've been pretty reluctant to add such an extension method so I don't think that they would be open to this version of it. You're free to roll your own, though.

[whoisj]

Could ForEach return Boolean to indicate if it managed to iterate any objects? This would enable if(!ForEach(....)){} syntax with nothing else changed.

In this case, we could just go full Rust and allow our iterator, switch, and other random control statements to return final values for assignment. 😏

Gotta love

var foo = switch (bar)
{
  case 1: return value1;
  case 2: return value2;
  default: return 0;
}

[MkazemAkhgary]

@whoisj and maybe try lock?

var entered = lock(obj)
{
      // statements
}

and

var succeed = try
{
}
catch
{
}

[whoisj]

I think I might need to start specifying #sarcasm on GitHub now too 😕

[tuespetre]

@whoisj 😏 and 🙄 are good enough 😛

[Pzixel]

@whoisj we're going to have match that is expression so it's not really a joke :)

@MkazemAkhgary

define overhead, Count from linq will cause duplicate enumeration. But Any does not. Most collections have Count property so you only check length. If that still is problem, which is very rare case, then you can use flag, as i suggested above.

Simple example:

var range = Enumerable.Range(1, 1_000_000_000).Where(e => e > 1_000_000_000 - 1);
int sum = -1;
if (range.Any())
{
   foreach(var x in range)
      sum += x;
}
else
{
   sum = 0;	
}

var range = Enumerable.Range(1, 1_000_000_000).Where(e => e > 1_000_000_000 - 1);
int sum = 0;
range.ForEach(x => sum += x, () => sum = 0);

The latter is 2x faster.

[Thaina]

@AartBluestoke Making it Action delegate cause an delegate overhead and make the code block cannot access ref argument

Until we have inline/sync delegate as I propose https://github.com/dotnet/roslyn/issues/11882 I still prefer actual foreach

[jimbobmcgee]

(Glad I found this -- have spent all evening trying to to find duplicates before raising it myself. It seemed too obvious to have not been raised before.)

In short, would love to see this implemented and extended to all the loop constructs, not just foreach.

Also recognise that using else (while the most natural keyword to my eyes) would conflict with unbraced if (...) foreach (...) { ... } statements, so proffer foreach (...) { } or { } instead. I also considered foreach (...) { } empty { } (although empty as keyword loses its semantics if not dealing with collections).

With regards to the other loop constructs, I could see the usefulness. The naive approaches are obvious, but I would prefer the most-optimal appraoches...

for|foreach|while (...) { } or { } roughly translates to...

{
    bool flag = false;
    for|foreach|while (...) { flag=true; ... }
    if (!flag) { ... }
}

do { } while (...) or { } is slightly less-obvious, as you would still want the first-iteration semantics of do/while to be honoured, and for or { } to only run if the second iteration didn't fire...

{
    int count = 0;
    do { ++count; ... } while (...);
    if (count == 1) { ... }
}

foreach is definitely where I can see the most benefit, though, especially with the possible side-effects of realising the enumerable.

[NineBerry]

@MkazemAkhgary:

Any may not cause complete enumeration, but the partial enumeration that occurs may incur some overhead -- whether that be reading from a file, a database, an Active Directory query, etc.

It's even possible that when the IEnumerable<> is actually an iterator method that determines the result set dynamically, the first enumeration could return a non-empty collection while the second one would then return an empty collection.

So, it could be possible that the foreach doesn't loop at all even though the Any() call has returned true before. A dedicated language feature of an or-clause for loops would be a clean way of avoiding multiple invocations of a generator.

In the same way, you sometimes want to do stuff before iterating only when the collection is not empty. Having an extension to the foreach-syntax that would allow code to execute before the loop or after the loop only when the loop is actually run once, would be very helpful in avoiding multiple invocations of generators.

foreach(var person in searchResult)
{
    Console.WriteLine(person.Name);
}
before
{
    Console.WriteLine("These persons were found:");
}
after
{
    Console.WriteLine("End of list");
}
or 
{
    Console.WriteLine("No persons were found");
}

which would be equivalent of

bool iterated = false;
foreach (var person in searchResult)
{
    if (!iterated)
    {
        Console.WriteLine("These persons were found:");
        iterated = true;
    }

    Console.WriteLine(person.Name);
}

if(iterated)
{
    Console.WriteLine("End of list");
}
else
{
    Console.WriteLine("No persons were found");
}

[Pzixel]

This feature is not worthy of 3 distinct new keywords and such radical syntax changes.