"using" statement as a part of "foreach"

[KorneiDontsov]

Description

What I have

interface IStuffAdapter : IDisposable
{
    // provides some stuff
}

...

IEnumerable<IStuffAdapter> stuffAdapterSeq = ...;
foreach (var adapter in stuffAdapterSeq)
{
    using var uselessVar = adapter;

   // get stuff
}

// or even
foreach (var adapter in stuffAdapterSeq)
{
    using (adapter)
    {
        // get stuff
    }
}

What I want to have

foreach (using var adapter in stuffAdapterSeq)
{
   // get stuff
}

This should also work with IAsyncEnumerable and IAsyncDisposable:

interface IStuffAdapter : IAsyncDisposable
{
    // provides some stuff
}

...

IAsyncEnumerable<IStuffAdapter> stuffAdapterSeq = ...;
await foreach (await using var adapter in stuffAdapterSeq)
{
    // get stuff
}

Usability

First case

I have an async stream of TextReaders which I want to deserialize and put to another async stream.

async IAsyncEnumerable<DeserializedObject> StreamDeserializedObjects(
    IAsyncEnumerable<TextReader> serializedObjectReaderStream)
{
    await foreach (using var reader in serializedObjectReaderStream)
    {
        yield return await this.Serializer.DeserializeAsync<DeserializedObject>(reader);
    }
}

TextReader is an abstract class so I cannot do it other way and keep polymorphism.

Second case

I have to do an sql query and return result as an async stream of rows.

interface ISqlQueryable
{
    IAsyncStream<ISqlRowReader> ExecuteAndStream(string query);
}

interface ISqlRowReader
{
    SqlType GetDataType(int columnPosition);

    string GetString(int columnPosition);

    // etc
}

...

ISqlQueryable sqlQueryable = ...;
var rowReaderStream = sqlQueryable.ExecuteAndStream("SELECT * FROM table1");
await foreach (var rowReader in rowReaderStream)
{
    // read row
}

— You're ignorant! You're going to allocate a new instance of ISqlRowReader for every row in result! Why don't you implement some interface kind of System.Data.IDataReader with a method kind of ValueTask ReadAsync() and iterate query result without allocating new objects?

You're right. But I can do the same using IAsyncEnumerable and polled objects:

// Instances of this interface are the pooled objects so you must dispose them after use.
interface ISqlRowReader : IDisposable
{
    SqlType GetDataType(int columnPosition);

    string GetString(int columnPosition);

    // etc
}

...
ISqlQueryable sqlQueryable = ...;
var rowReaderStream = sqlQueryable.ExecuteAndStream("SELECT * FROM table1");
await foreach (using var rowReader in rowReaderStream)
{
    // read row

   // rowReader returns back to pool so it can be reused in next iteration
}

Now I have only one extra allocation but I keep using await foreach which is so fluent and readable!

So

There are the patterns required to enumerate and stream the disposable objects and it would be great to iterate them in a fluent way. My cases seems very common for me as a backend programmer. "using" statement as a part of "foreach" will keep IAsyncEnumerable as a "chosen one" abstraction for an async streams.

[YairHalberstadt]

foreach (var adapter in stuffAdapterSeq)
{
    using (adapter)
    {
        // get stuff
    }
}

I don't see what's wrong with this?

[KorneiDontsov]

For me it's just ugly. Usage of a disposable object looks really good and safe only if you assign it to a local variable right into the using statement.

[YairHalberstadt]

Usage of a disposable object looks really good and safe only if you assign it to a local variable right into the using statement.

Why?

[KorneiDontsov]

Usage of a disposable object looks really good and safe only if you assign it to a local variable right into the using statement.

Why?

Because you cannot use a variable defined into using statement out of scope. I quess this is the main point of using keyword in C#.

[HaloFour]

This is already legal. You may have to fight with the autoformatter, though. I haven't tried in VS. If that's the case you could post an issue in the Roslyn repo to have the formatter respect it.

foreach (var adapter in stuffAdapterSeq) using (adapter)
{
    // get stuff
}

[KorneiDontsov]

This is a good point. I can live with this syntax even if I believe my syntax is better :)

I want to say that from the very beginning using is not about a feature. It's just a syntax sugar. So I think the question is not about the ways I can implement my ideas. It's about how far can we go to write common things shorter. That's why I proposed new syntax.
If you think that we have gone far enough then I'll create issue in the Roslyn repo.

[canton7]

It's about how far can we go to write common things shorter.

I disagree. It's about readability and maintainability. Good code isn't about how few keystrokes it takes to write, but about how easy it is for someone who's never seen it before to understand and fix it.

Sometimes that's the same as "shorter" (by reducing code clutter and boilerplate), but sometimes it isn't. In this case, you're introducing syntax that every C# developer will have to learn, in order to make a rare case a few characters terser. You're also IMO making it less maintainable, as it's less obvious to someone skimming over the code that the loop variable is being disposed.

Remember that disposal in particular is something that developers need to keep track of by hand, and it's often not obvious when something is not being disposed when it should, or is being disposed twice. Making this information harder to find is not going to help.

[TheFanatr]

It's about readability and maintainability. Good code isn't about how few keystrokes it takes to write, but about how easy it is for someone who's never seen it before to understand and fix it.

While I agree with this, I believe that if the syntax for expressing a certain operation can be easily made more elegant, it should be, as it would not necessarily hurt readability as long as the language is understood, but would make the code more clean and faster to write. An example of this in practice is the ?? operator.

Remember that disposal in particular is something that developers need to keep track of by hand, and it's often not obvious when something is not being disposed when it should, or is being disposed twice. Making this information harder to find is not going to help.

I find the use of this argument odd, as the using block isn't necessarily clear in it's functionality in the first place, and with the introduction of the using statement in C# 8, it is already even less obvious; at this point, allowing the using keyword in front of any variable declaration wouldn't make much of a difference, but would help with code cleanliness.

[john-h-k]

Old issue, but want to add

foreach (var adapter in stuffAdapterSeq) using (adapter)
{
    // get stuff
}

and

foreach (var adapter in stuffAdapterSeq)
{
    using (adapter)
    {
        // get stuff
    }
}

are not ok when adapter is a value type, because using copies

[CyrusNajmabadi]

@johnkellyoxford You can. That's already legal today in C#

are not ok when adapter is a value type, because using copies

This seems to be a separate request.

[john-h-k]

Yes, you can do it. But it copies, which will often yield unexpected results when used with a value type

[canton7]

@johnkellyoxford What copy are you worried about, exactly?

The foreach loop will copy value types, yes, but that will happen without the using statement. The using statement itself won't create a copy.

[theraot]

I just wrote a foreach (using var stream in GetStreams()){ /*...*/ } expecting it to work. If anything, it has the principle of least astonishment to its favor.

[Ilia-Kosenkov]

It looks like the issue is quite old, but I just encountered a problem described by the author.
Using using statement in foreach body explicitly may work, but, I believe, the idea is to allow the compiler to lower the code properly, something like from this

foreach(using var item in collection)
{/* working with disposable item */}

to something like this (and also unwrapping using to try-finally)

using(var enumer = collection.GetEnumerator())
    while(enumer.MoveNext())
        using(var item = enumer.Current)
        {/* working with disposable item */}

This allows for all the optimizations to happen accordingly, and also gives a hint to the compiler about resource usage within loop body.
Providing the scope of the foreach loop body is well-defined, foreach(using var...) is even more clear than using var item ... statement introduced in C#8.

However, the downside of this issue is that the collection item can support IAsyncDisposable and it can be the preferred way of handling associated with item resources. This can lead to structs like

await foreach(await using var item in collection)
{/* get item asynchronously, do work and dispose asynchronously */}

[AlgorithmsAreCool]

I am in favor of a foreach(using var ...) syntax as proposed.

The existing suggested syntax foreach (var adapter in stuffAdapterSeq) using (adapter) is problematic if you auto-format your code alot.

The visual studio formatter gets confused by it and indents the braces 'incorrectly'.

//before format document
foreach (var container in site.GetFileContainers()) using (container)
{
    //stuff
}

//after format document
foreach (var container in site.GetFileContainers()) using (container)
    {
        //stuff
    }

[333fred]

@AlgorithmsAreCool if you want to see a formatting change, please suggest it on dotnet/roslyn.

[AlgorithmsAreCool]

@333fred
I'm not asking for a formatting change, i am arguing for proper language support of this scenario so the formatter's can behave normally.

[yaakov-h]

not the best justification in the world... 😉

[CyrusNajmabadi]

I'm not asking for a formatting change, i am arguing for proper language support of this scenario so the formatter's can behave normally.

That's not a language change. Even if the language supports something, the formatter only changes by fixing up the formatter.

[AlgorithmsAreCool]

I seem to making terrible comments all over the place today :/
@333fred I'm sorry if my response was unnecessarily rude, I was careless when I wrote it.

@CyrusNajmabadi
The point I was trying (poorly) to make is that we would have to fight normal formatters to use that syntax which limits it's usability.

This 'existing' syntax (foreach(...) using(...)) is using the 'single-line' version of the foreach statement in a clever way. But most/all existing formatters would typically wrap and indent it automatically. Or they might expect it to be a one-liner like foreach(...) using() { ... } which might be similarly undesirable.

Edit: Re-reading the thread shows that Halofour made this same observation months ago. ☹

[CyrusNajmabadi]

But most/all existing formatters would typically wrap and indent it automatically.

So fix them. :-)

It's roughly 1000x-10000x cheaper than updating the language. (Note: that's not an exaggeration on costs).

[AlgorithmsAreCool]

So fix them. :-)

@CyrusNajmabadi
I personally feel the language would be a better arbiter of this pattern. Not a wilderness of style guides and formatter implementations. I'm assuming others would disagree with me.

But it boils down to this for me,
I reached into the C# toolbox today for something relatively simple I expected to be there and was mildly surprised to find that it didn't exist. I can use using var in most other declarations, it seemed natural to me to be able to use them in a foreach loop. It saves a level of brace nesting and honestly feels like an ideal use case for using var since you can reuse the existing declaration scope.

Maybe it's fluff, maybe it's a little usability gap, but it surprised me.