Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Proposal] IDisposable can be used with tuple #1898

Closed
hez2010 opened this issue Sep 30, 2018 · 8 comments
Closed

[Proposal] IDisposable can be used with tuple #1898

hez2010 opened this issue Sep 30, 2018 · 8 comments

Comments

@hez2010
Copy link

hez2010 commented Sep 30, 2018

Assuming we have multiple object which can be disposed.

StreamReader a, b, c, ....;
.....

In order to dispose them, currently we have to dispose them manually separately or use plenty of nested 'using' blocks.
Both of ways mentioned above will cause some problem:

  1. we need dispose them in all possible code branches which may lead to bugs.
....
if (...)
{
    a?.Dispose();
    b?.Dispose();
    ....
}
else
{
    a?.Dispose();
    b?.Dispose();
    ....
}
.....
try
{
   ....
}
finally
{
    c?.Dispose();
}
....
  1. a lot of nested using blocks which makes code ugly and unreadable.
using (a)
{
    using (b)
    {
        using (c)
        {
            ....
        }
    }
}

I think the best coding style is:

using ((a, b, c))
{
    ....
}

using (var (a, b, c) = (new StreamReader(), new StreamReader(), new StreamReader()))
{
    ....
}

We make them a tuple and use a using block. And when we are out of the using block, all of them will be disposed by order automatically.

And I think the disposing order is important.
For instance, we have a FileStream x and a StreamReader y which is created from x, then y should be disposed before x, so we should write:

using ((y, x))
{
    ....
}
@yaakov-h
Copy link
Member

Your second case seems over-exaggerated. You can do this today:

using (a)
using (b)
using (c)
{
    ....
}

I personally don't find that to be ugly or unreadable.

@hez2010
Copy link
Author

hez2010 commented Sep 30, 2018

@yaakov-h
If you apply code clean up, it will become:

using (a)
    using (b)
        using (c)
        {
            ....
        }

@yaakov-h
Copy link
Member

I dunno what refactoring tool you're using, but VS2017 15.8.5 is quite happy with what I posted, and actually de-indents the subsequent using statements as I type.

After applying Format Document, it doesn't re-indent them.

Most formatting analyzers I've seen such as StyleCop are perfectly happy with directly nested usings sharing a common indentation level.

However, if you're happy with the example I posted, then you should fix your broken tooling or file a bug report. We don't need the language to be changed when it is already satisfactory.

@Austin-bryan
Copy link

Cool idea, but why not just remove the unnecessary parens and do this:

using (a, b, c)
{
    ....
}

Which is the same as:

using (a)
{
    using (b)
    {
        using (c)
        {
            ....
        }
    }
}

But there's no tuples at all, just syntactic sugar. This also allows for a, b, and c to be of different types.

@svick
Copy link
Contributor

svick commented Sep 30, 2018

This looks like a duplicate of #1473.

@theunrepentantgeek
Copy link

With #1623, the existing using statement will check for extension methods, so you'd then be able to write this:

public static class TupleExtensions
{
    public static void Dispose<X,Y>(this (X x, Y y) tuple)
        where X : IDisposable, Y : IDisposable
    {
        x.Dispose();
        y.Dispose();
    }
}

That proposal is already flagged for the v8.0 milestone, so it's very likely to happen - and it would make this proposal obsolete.

See also #1703 for more details.

@theunrepentantgeek
Copy link

There's also #1174, proposing a new version of using where the existing scope is used instead of creating a new one.

This would eliminate the nesting you assert makes code ugly and unreadable:

{
    StreamReader a, b, c, ....;

    using a = new StreamReader(...);
    using b = new StreamReader(...);
    using c = new StreamReader(...);
}

#1174 is also tagged for the v8.0 milestone and is therefore very likely to happen.

@YairHalberstadt
Copy link
Contributor

Closing as duplicate of #1473

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants