C# conditional using block statement

Question

I have the follow code but it is awkward. How could I better structure it? Do I have to make my consuming class implement IDisposable and conditionally construct the network access class and dispose it when I am done?

    protected void ValidateExportDirectoryExists()
    {
        if (useNetworkAccess)
        {
            using (new Core.NetworkAccess(username, password, domain))
            {
                CheckExportDirectoryExists();
            }
        }
        else
        {
            CheckExportDirectoryExists();
        }
    }

@Joel Etherton: Probably because of the repetition of that `CheckExportDirectoryExists()` call. — BoltClock, Dec 08 '10 at 16:20
If that is the most awkward bit of your code, you're very well off. — Nate, Dec 08 '10 at 16:22

Jon Skeet · Accepted Answer · 2022-10-01T16:02:27.323

90

One option, which is somewhat nasty but would work, based on the fact that the C# compiler calls Dispose only if the resource is non-null:

protected void ValidateExportDirectoryExists()
{
    using (useNetworkAccess 
               ? new Core.NetworkAccess(username, password, domain)
               : null)
    {
        CheckExportDirectoryExists();
    }
}

Another alternative would be to write a static method which returned either null or a NetworkAccess:

private Core.NetworkAccess CreateNetworkAccessIfNecessary()
{
    return useNetworkAccess
        ? new Core.NetworkAccess(username, password, domain)) : null;
}

Then:

protected void ValidateExportDirectoryExists()
{
    using (CreateNetworkAccessIfNecessary())
    {
        CheckExportDirectoryExists();
    }
}

Again, I'm still not sure I don't prefer the original... it really depends on how often you need this pattern.

edited Oct 01 '22 at 16:02

answered Dec 08 '10 at 16:20

Jon Skeet

1,421,763
867
9,128
9,194

1

You have the logic there backwards. Swap `null` and the `new` expression. – cdhowie Dec 08 '10 at 16:22
Original is prettier, but +1 anyway :) – Daniel Mošmondor Dec 08 '10 at 16:23
2

I didn't know that about using null in the using statement. – wageoghe Dec 08 '10 at 16:28
@John: I probably wouldn't have done in VS, but I don't like code wrapping on Stack Overflow, which has short lines. – Jon Skeet Dec 08 '10 at 16:32
+1 I actually like this a lot. Didn't consider using 'null' before for this, it actually makes things clearer in my situation here because the method call has a bunch of parameters. – julealgon Jul 09 '13 at 15:23
what if the resource is a struct...? – Logerfo Jun 09 '17 at 17:32
@Logerfo: I don't have a specific answer for that, but it's a pretty niche corner case... – Jon Skeet Jun 09 '17 at 17:33
Cannot edit due to "Suggested edit queue is full" but the docs link has changed into: https://learn.microsoft.com/en-us/dotnet/csharp/language-reference/language-specification/statements#1214-the-using-statement – joakimriedel Oct 01 '22 at 15:45

score 15 · Answer 2 · answered Dec 08 '10 at 17:02

15

The using statement is a shortcut to avoid "finally" blocks and should only be used when it makes the code easier to follow. In your case I would write the following code. It may not be as brief as some of the other versions, but is much more straight forward.

protected void ValidateExportDirectoryExists()
{
    Core.NetworkAccess access = useNetworkAccess ? new Core.NetworkAccess(username, password, domain) : null;    

    try
    {
        CheckExportDirectoryExists()
    }
    finally
    {
       if (access != null)
       {
           access.Dispose();
       }
    }
}

answered Dec 08 '10 at 17:02

Dmitry S.

8,373
2
39
49

I would agree, I think this is more intention revealing than the using statement - just because it's immediately obvious that access is optional, because of that null check in the finally statement - where as a using statement obfuscates the intention a little. – Bittercoder Dec 09 '10 at 12:52
i think this is the best answer only because it does the best job of showing intent while doing what is needed without being "clever" in any way. this would be the idiomatic C# way imo. – Mike Corcoran May 09 '17 at 13:58
You are initializing your disposable resource outside of the try/finally. This is clearly not a good idea. – rold2007 Apr 12 '20 at 23:33
If the object's constructor fails you will not be able to dispose it. Here is a link from the Microsoft site how the using statement is implemented behind the scenes: https://learn.microsoft.com/en-us/dotnet/csharp/language-reference/language-specification/statements#the-using-statement – Dmitry S. Apr 17 '20 at 02:14

score 6 · Answer 3 · answered Dec 08 '10 at 16:23

6

If you repeat this pattern in many methods you can break out the pattern

protected void OptionalNetworkCall(Action action)
{
    if (useNetworkAccess)
    {
        using (new Core.NetworkAccess(username, password, domain))
        {
            action();
        }
    }
    else
    {
        action();
    }
}

protected void ValidateExportDirectoryExists()
{
    OptionalNetworkCall(CheckExportDirectoryExists);
}

answered Dec 08 '10 at 16:23

Albin Sunnanbo

46,430
8
69
108

1

This is, most likely, precisely what OP wanted to avoid. – Dmitri Nesteruk Dec 03 '19 at 12:40

score 2 · Answer 4 · answered Dec 08 '10 at 16:31

2

protected void ValidateExportDirectoryExists()
{
      var access = useNetworkAccess
          ? new Core.NetworkAccess(username, password, domain)
            : null;

      using (access)
      {
          CheckExportDirectoryExists();
      }
}

answered Dec 08 '10 at 16:31

Jay

56,361
10
99
123

score 1 · Answer 5 · answered Dec 08 '10 at 16:27

I don't know if it is "better", but you could use the null object pattern and have a "null" disposable network access object. Something like this:

protected void ValidateExportDirectoryExists()     
{
  using (GetNetworkAccess(username, password, domain))
  {                 
    CheckExportDirectoryExists();
  }
} 

protected IDisposable GetNetworkAccess(string username, string password, string domain)
{
  return useNetworkAccess ? new Core.NetworkAccess(username, password, domain) : new NullNetworkAccess(username, password, domain);
}

internal class NullNetworkAccess : IDisposable
{
  internal NullNetworkAccess(string username, string password, string domain)
  {
  }

  public void Dispose()
  {
  }
}

This is probably too cute for its own good.

[EDIT] Just saw in Jon's answer that null can be used in a using statement. I had no idea!

score 0 · Answer 6 · answered Dec 08 '10 at 16:20

0

using scope will only dispose a object if the class implements IDisposible interface so yes you need to implement dispose method.

answered Dec 08 '10 at 16:20

TalentTuner

17,262
5
38
63

score 0 · Answer 7 · answered Dec 08 '10 at 16:21

0

I guess that is really a matter of cosmetics if the code is as simple as that.

I can envision how it could look the other way, and my vote will be for this version you have now.

answered Dec 08 '10 at 16:21

Daniel Mošmondor

19,718
12
58
99

score 0 · Answer 8 · answered Dec 08 '10 at 16:22

Whatever is enclosed within the using statement will have it's IDispoable.Dispose called as dictated by the IDisposable interface. As seen on MSDN for using...

Provides a convenient syntax that ensures the correct use of IDisposable objects.

Therefore if you put a custom type within the using statement it should clean up its resources appropriately via the IDisposable interface.

score 0 · Answer 9 · answered Dec 08 '10 at 16:24

By having your class implement IDisposable, the dispose method is called only when using the "using" statement. Otherwise you have to explicitly call dispose.

Typically IDisposable is implemented by objects that manage memory consumption outside of the garbage collector (like using unmanaged code for example). It provides a way to clean up any consumed memory.

So long as your NetworkAccess class implements IDisposable, the dispose method will get called as soon as the scope of the using statement is complete. If it is managed code, then no need to dispose of it. Just let the garbage collector do its work.

score 0 · Answer 10 · answered Dec 08 '10 at 16:41

Use your own try/finally block, which performs similar logic to the 'using', but only does the dispose if useNetworkAccess is set. Note that if useNetworkAccess could be affected by other threads, you should copy its value and use that copy both for creating the resource and disposing it.

C# conditional using block statement

10 Answers10

Linked