SBurris
SBurris

Reputation: 7448

Nested using statements in C#

I am working on a project. I have to compare the contents of two files and see if they match each other precisely.

Before a lot of error-checking and validation, my first draft is:

  DirectoryInfo di = new DirectoryInfo(Environment.CurrentDirectory + "\\TestArea\\");
  FileInfo[] files = di.GetFiles(filename + ".*");

  FileInfo outputFile = files.Where(f => f.Extension == ".out").Single<FileInfo>();
  FileInfo expectedFile = files.Where(f => f.Extension == ".exp").Single <FileInfo>();

  using (StreamReader outFile = new StreamReader(outputFile.OpenRead()))
  {
    using (StreamReader expFile = new StreamReader(expectedFile.OpenRead()))
    {
      while (!(outFile.EndOfStream || expFile.EndOfStream))
      {
        if (outFile.ReadLine() != expFile.ReadLine())
        {
          return false;
        }
      }
      return (outFile.EndOfStream && expFile.EndOfStream);
    }
  }

It seems a little odd to have nested using statements.

Is there a better way to do this?

Upvotes: 367

Views: 123614

Answers (17)

SLaks
SLaks

Reputation: 887215

Also, if you already know the paths, there's no point is scanning the directory.

Instead, I would recommend something like this:

string directory = Path.Combine(Environment.CurrentDirectory, @"TestArea\");

using (StreamReader outFile = File.OpenText(directory + filename + ".out"))
using (StreamReader expFile = File.OpenText(directory + filename + ".exp")) 
{
    //...

Path.Combine will add a folder or filename to a path and make sure that there is exactly one backslash between the path and the name.

File.OpenText will open a file and create a StreamReader in one go.

By prefixing a string with @, you can avoid having to escape every backslash (eg, @"a\b\c")

Upvotes: 6

obelix
obelix

Reputation: 986

These come up time to time when I code as well. You could consider move the second using statement into another function.

Upvotes: 3

womp
womp

Reputation: 116977

There's nothing odd about it. using is a shorthand way of ensuring the disposal of the object once the code block is finished. If you have a disposable object in your outer block that the inner block needs to use, this is perfectly acceptable.

Upvotes: 5

user11523568
user11523568

Reputation:

Since C# 8.0 you can use a using declaration.

using var outFile = new StreamReader(outputFile.OpenRead());
using var expFile = new StreamReader(expectedFile.OpenRead());
while (!(outFile.EndOfStream || expFile.EndOfStream))
{
    if (outFile.ReadLine() != expFile.ReadLine())
    {
         return false;
    }
}
return (outFile.EndOfStream && expFile.EndOfStream);

This will dispose of the using variables in the end of the scope of the variables, i.e. in the end of the method.

Upvotes: 28

Rajan Mishra
Rajan Mishra

Reputation: 1178

Its the normal way of use and works perfect. Although there are some other ways of implementing this. Almost every answer is already present in this question's response. But here I am listing all of them together.

Already Used

using (StreamReader outFile = new StreamReader(outputFile.OpenRead()))
  {
    using (StreamReader expFile = new StreamReader(expectedFile.OpenRead()))
    {
      while (!(outFile.EndOfStream || expFile.EndOfStream))
      {
        if (outFile.ReadLine() != expFile.ReadLine())
        return false;
      }
    }
  }

Option 1

using (StreamReader outFile = new StreamReader(outputFile.OpenRead()))
    using (StreamReader expFile = new StreamReader(expectedFile.OpenRead()))
    {
      while (!(outFile.EndOfStream || expFile.EndOfStream))
      {
        if (outFile.ReadLine() != expFile.ReadLine())
        return false;
      }
    }
  }

Option 2

using (StreamReader outFile = new StreamReader(outputFile.OpenRead()),
                    expFile = new StreamReader(expectedFile.OpenRead()))
   {
      while (!(outFile.EndOfStream || expFile.EndOfStream))
       {
         if (outFile.ReadLine() != expFile.ReadLine())
         return false;
       }
    }

Upvotes: 3

Gavin H
Gavin H

Reputation: 10482

If the objects are of the same type you can do the following

using (StreamReader outFile = new StreamReader(outputFile.OpenRead()), 
                    expFile = new StreamReader(expectedFile.OpenRead()))
{
    // ...
}

Upvotes: 152

hcp
hcp

Reputation: 486

I think I may have found a syntactically cleaner way of declaring this using statement, and it appears to work for me? using var as your type in the using statement instead of IDisposable seems to dynamically infer type on both objects and allows me to instantiate both of my objects and call their properties and methods of the class they are allocated with, as in

using(var uow = new UnitOfWorkType1(), uow2 = new UnitOfWorkType2()){}.

If anyone knows why this isn't right, please let me know

Upvotes: 2

codymanix
codymanix

Reputation: 29468

You can group multiple disposable objects in one using-statement with commas:

using (StreamReader outFile = new StreamReader(outputFile.OpenRead()), 
       expFile = new StreamReader(expectedFile.OpenRead()))
{

}

Upvotes: 7

jpierson
jpierson

Reputation: 17356

The using statement works off of the IDisposable interface so another option could be to create some type of composite class that implements IDisposable and has references to all of the IDisposable objects you would normally put in your using statement. The down side to this is that you have to declare your variables first and outside of the scope for them to be useful within the using block requiring more lines of code than some of the other suggestions would require.

Connection c = new ...; 
Transaction t = new ...;

using (new DisposableCollection(c, t))
{
   ...
}

The constructor for DisposableCollection is a params array in this case so you can feed in as many as you like.

Upvotes: 11

Botz3000
Botz3000

Reputation: 39600

if you don't mind declaring the variables for your using block before the using block, you could declare them all in the same using statement.

    Test t; 
    Blah u;
    using (IDisposable x = (t = new Test()), y = (u = new Blah())) {
        // whatever...
    }

That way, x and y are just placeholder variables of type IDisposable for the using block to use and you use t and u inside your code. Just thought i'd mention.

Upvotes: 18

SLaks
SLaks

Reputation: 887215

The preferred way to do this is to only put an opening brace { after the last using statement, like this:

using (StreamReader outFile = new StreamReader(outputFile.OpenRead()))
using (StreamReader expFile = new StreamReader(expectedFile.OpenRead())) 
{
    ///...
}

Upvotes: 636

Charles Bretana
Charles Bretana

Reputation: 146409

And to just add to the clarity, in this case, since each successive statement is a single statement, (and not a block), you can omit all the brackets :

using (StreamReader outFile = new StreamReader(outputFile.OpenRead()))
  using (StreamReader expFile = new StreamReader(expectedFile.OpenRead()))
    while (!(outFile.EndOfStream || expFile.EndOfStream))  
       if (outFile.ReadLine() != expFile.ReadLine())    
          return false;  

Upvotes: 5

TimothyP
TimothyP

Reputation: 21745

Are you also asking if there is a better way to compare to files? I prefer calculating a CRC or MD5 for both files and compare those.

For example you could use the following extension method:

public static class ByteArrayExtender
    {
        static ushort[] CRC16_TABLE =  { 
                      0X0000, 0XC0C1, 0XC181, 0X0140, 0XC301, 0X03C0, 0X0280, 0XC241, 
                      0XC601, 0X06C0, 0X0780, 0XC741, 0X0500, 0XC5C1, 0XC481, 0X0440, 
                      0XCC01, 0X0CC0, 0X0D80, 0XCD41, 0X0F00, 0XCFC1, 0XCE81, 0X0E40, 
                      0X0A00, 0XCAC1, 0XCB81, 0X0B40, 0XC901, 0X09C0, 0X0880, 0XC841, 
                      0XD801, 0X18C0, 0X1980, 0XD941, 0X1B00, 0XDBC1, 0XDA81, 0X1A40, 
                      0X1E00, 0XDEC1, 0XDF81, 0X1F40, 0XDD01, 0X1DC0, 0X1C80, 0XDC41, 
                      0X1400, 0XD4C1, 0XD581, 0X1540, 0XD701, 0X17C0, 0X1680, 0XD641, 
                      0XD201, 0X12C0, 0X1380, 0XD341, 0X1100, 0XD1C1, 0XD081, 0X1040, 
                      0XF001, 0X30C0, 0X3180, 0XF141, 0X3300, 0XF3C1, 0XF281, 0X3240, 
                      0X3600, 0XF6C1, 0XF781, 0X3740, 0XF501, 0X35C0, 0X3480, 0XF441, 
                      0X3C00, 0XFCC1, 0XFD81, 0X3D40, 0XFF01, 0X3FC0, 0X3E80, 0XFE41, 
                      0XFA01, 0X3AC0, 0X3B80, 0XFB41, 0X3900, 0XF9C1, 0XF881, 0X3840, 
                      0X2800, 0XE8C1, 0XE981, 0X2940, 0XEB01, 0X2BC0, 0X2A80, 0XEA41, 
                      0XEE01, 0X2EC0, 0X2F80, 0XEF41, 0X2D00, 0XEDC1, 0XEC81, 0X2C40, 
                      0XE401, 0X24C0, 0X2580, 0XE541, 0X2700, 0XE7C1, 0XE681, 0X2640, 
                      0X2200, 0XE2C1, 0XE381, 0X2340, 0XE101, 0X21C0, 0X2080, 0XE041, 
                      0XA001, 0X60C0, 0X6180, 0XA141, 0X6300, 0XA3C1, 0XA281, 0X6240, 
                      0X6600, 0XA6C1, 0XA781, 0X6740, 0XA501, 0X65C0, 0X6480, 0XA441, 
                      0X6C00, 0XACC1, 0XAD81, 0X6D40, 0XAF01, 0X6FC0, 0X6E80, 0XAE41, 
                      0XAA01, 0X6AC0, 0X6B80, 0XAB41, 0X6900, 0XA9C1, 0XA881, 0X6840, 
                      0X7800, 0XB8C1, 0XB981, 0X7940, 0XBB01, 0X7BC0, 0X7A80, 0XBA41, 
                      0XBE01, 0X7EC0, 0X7F80, 0XBF41, 0X7D00, 0XBDC1, 0XBC81, 0X7C40, 
                      0XB401, 0X74C0, 0X7580, 0XB541, 0X7700, 0XB7C1, 0XB681, 0X7640, 
                      0X7200, 0XB2C1, 0XB381, 0X7340, 0XB101, 0X71C0, 0X7080, 0XB041, 
                      0X5000, 0X90C1, 0X9181, 0X5140, 0X9301, 0X53C0, 0X5280, 0X9241, 
                      0X9601, 0X56C0, 0X5780, 0X9741, 0X5500, 0X95C1, 0X9481, 0X5440, 
                      0X9C01, 0X5CC0, 0X5D80, 0X9D41, 0X5F00, 0X9FC1, 0X9E81, 0X5E40, 
                      0X5A00, 0X9AC1, 0X9B81, 0X5B40, 0X9901, 0X59C0, 0X5880, 0X9841, 
                      0X8801, 0X48C0, 0X4980, 0X8941, 0X4B00, 0X8BC1, 0X8A81, 0X4A40, 
                      0X4E00, 0X8EC1, 0X8F81, 0X4F40, 0X8D01, 0X4DC0, 0X4C80, 0X8C41, 
                      0X4400, 0X84C1, 0X8581, 0X4540, 0X8701, 0X47C0, 0X4680, 0X8641, 
                      0X8201, 0X42C0, 0X4380, 0X8341, 0X4100, 0X81C1, 0X8081, 0X4040 };


        public static ushort CalculateCRC16(this byte[] source)
        {
            ushort crc = 0;

            for (int i = 0; i < source.Length; i++)
            {
                crc = (ushort)((crc >> 8) ^ CRC16_TABLE[(crc ^ (ushort)source[i]) & 0xFF]);
            }

            return crc;
        }

Once you've done that it's pretty easy to compare files:

public bool filesAreEqual(string outFile, string expFile)
{
    var outFileBytes = File.ReadAllBytes(outFile);
    var expFileBytes = File.ReadAllBytes(expFile);

    return (outFileBytes.CalculateCRC16() == expFileBytes.CalculateCRC16());
}

You could use the built in System.Security.Cryptography.MD5 class, but the calculated hash is a byte[] so you'd still have to compare those two arrays.

Upvotes: 3

Jason Williams
Jason Williams

Reputation: 57892

If you want to compare the files efficiently, don't use StreamReaders at all, and then the usings aren't necessary - you can use low level stream reads to pull in buffers of data to compare.

You can also compare things like the file size first to quickly detect different files to save yourself having to read all the data, too.

Upvotes: 8

jason
jason

Reputation: 241583

When the IDisposables are of the same type, you can do the following:

 using (StreamReader outFile = new StreamReader(outputFile.OpenRead()), 
     expFile = new StreamReader(expectedFile.OpenRead()) {
     // ...
 }

The MSDN page on using has documentation on this language feature.

You can do the following whether or not the IDisposables are of the same type:

using (StreamReader outFile = new StreamReader(outputFile.OpenRead()))
using (StreamWriter anotherFile = new StreamReader(anotherFile.OpenRead()))
{ 
     // ...
}

Upvotes: 40

aquinas
aquinas

Reputation: 23786

You can also say:

using (StreamReader outFile = new StreamReader(outputFile.OpenRead()))
using (StreamReader expFile = new StreamReader(expectedFile.OpenRead()))
{
   ...
}

But some people might find that hard to read. BTW, as an optimization to your problem, why dont you check that the file sizes are the same size first, before going line by line?

Upvotes: 7

yoyoyoyosef
yoyoyoyosef

Reputation: 7228

You could omit the brackets on all but the inner-most using:

using (StreamReader outFile = new StreamReader(outputFile.OpenRead()))
using (StreamReader expFile = new StreamReader(expectedFile.OpenRead()))
{
  while (!(outFile.EndOfStream || expFile.EndOfStream))
  {
    if (outFile.ReadLine() != expFile.ReadLine())
    {
      return false;
    }
  }
}

I think this is cleaner than putting several of the same type in the same using, as others have suggested, but I'm sure many people will think this is confusing

Upvotes: 6

Related Questions