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

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.

About this issue

  • Original URL
  • State: closed
  • Created 7 years ago
  • Reactions: 6
  • Comments: 17 (9 by maintainers)

Most upvoted comments

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
    }

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

@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.

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