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)
Even if reconsidered I think that there are ambiguity concerns to consider. The following is already legal syntax:
We should just let else + any keywords format in the same line. And you should swap logic for empty on top
It already valid syntax (need to add
IsNullOrEmpty
extention method) just formatter make it annoying separate line@eyalsk
Correct, it would go in a different repo.
@jnm2 Oops, typo. #134 (I’ll fix the original comment too)