362

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?

geizio
  • 105
  • 1
  • 16
SBurris
  • 7,378
  • 5
  • 28
  • 36
  • 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 allow 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 = UnitOfWorkType1(), uow2 = UnitOfWorkType2()){} – hcp Feb 21 '14 at 15:58
  • Possible duplicate of [Dealing with nested "using" statements in C#](http://stackoverflow.com/questions/19217734/dealing-with-nested-using-statements-in-c-sharp) – 200_success Jun 10 '16 at 16:33
  • @200_success This question was asked in 2009 and that one in 2013, so I would probably flip the duplication identification if anything. (2¢, fyi, etc) – ruffin Sep 28 '21 at 20:05

17 Answers17

628

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())) 
{
    ///...
}
SLaks
  • 868,454
  • 176
  • 1,908
  • 1,964
  • 1
    Wow I learned something today. You can use this approach with different type. +1 – Pierre-Alain Vigeant Aug 25 '09 at 17:38
  • Yeah that approach is nice when the types are different. – Gavin H Aug 25 '09 at 17:39
  • 12
    Cleaner? and also doesn't force you to use the same types.. I always do it this way even if the types match for readability and consistency. – meandmycode Aug 25 '09 at 17:49
  • 3
    even if your going to trim out the brackets, for sake of convention I recommend you keep the indent on the nested portion – Hardryv Aug 25 '09 at 18:24
  • 7
    @Hardryv: Visual Studio's auto-format removes it. The idea is to look like a list of variable declarations. – SLaks Aug 25 '09 at 20:00
  • 49
    Not sure if I find that more readable at all. If anything it breaks the look of nested code. And it looks as if the first using statement is empty and unused. But, I guess what ever works... :/ – Jonathon Watney Aug 25 '09 at 20:07
  • 10
    @Bryan Watts, the "contrarians" may be expressing real preferences. It's very likely that a different group of devs would've dissented had nesting been recommended. The only way to know is to run the experiment again in a parallel universe. – Dan Rosenstark Aug 16 '11 at 23:37
  • 2
    CAVEAT: This does not work when passing a stream object to the StreamReader, because the StreamReader will dispose the stream as well ! See http://msdn.microsoft.com/de-de/library/ms182334.aspx – fmuecke Mar 08 '12 at 12:49
  • 6
    @fmuecke: That's not very true; it will work. The rules for `IDisposable` state that calling `Dispose()` twice should do nothing. That rule is only in case of poorly-written disposables. – SLaks Mar 08 '12 at 15:04
  • Perfect! Is this by design? The fact that visual studio's auto-format removes the nesting tells me that it is. – Connell Mar 21 '13 at 17:07
  • @ConnellWatkins: You can do the same thing with `if`, `for`, or `while`, except that it will indent. Specifically, you can use a single statement instead of a block, and `using` is a single statement. – SLaks Mar 21 '13 at 17:44
  • 1
    @SLaks, yes I am well aware of this (hence I'm asking, because of the difference in the way visual studio nests them). Anyway that is not true for all code blocks.. what about `try`, `catch` and `finally`? – Connell Mar 22 '13 at 09:54
  • 1
    @ConnellWatkins: Those require braces. http://ericlippert.com/2012/12/04/why-are-braces-required/ – SLaks Mar 22 '13 at 17:45
  • 2
    @SLaks Jesus Christ. Yes I know.. Rhetorical question to point out that you can't use a single statement in place of any old block (as you said). Let me rephrase - Do we think multiple using statements were **supposed** to be written like this as per a design decision by someone in the C# team at Microsoft? Visual Studio's different approach to nesting these would indicate that that is true. – Connell Mar 22 '13 at 18:18
  • @ConnellWatkins: I strongly assume so, unless you think that the editor feature happened entirely by accident – SLaks Mar 22 '13 at 19:14
  • Nope. I've made the same assumption (as I've said twice already). I'm after some proof or documentation, not just assumptions really. No worries, thanks anyway. – Connell Mar 25 '13 at 09:37
  • With this I would have to suppress the SA1503 rule of stylecop – John Demetriou Feb 19 '19 at 16:59
152

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()))
{
    // ...
}
budi
  • 6,351
  • 10
  • 55
  • 80
Gavin H
  • 10,274
  • 3
  • 35
  • 42
  • 1
    Well they are all the same type if they are all IDisposable perhaps a cast would work? – jpierson Jan 17 '12 at 17:43
  • 9
    @jpierson that does work, yes, but then when you're calling the `IDisposable` objects from inside the using block, we can't call any of the class members (without a cast, which defeats the point imo). – Connell Mar 22 '13 at 09:58
  • IDisposable is a type so just use it as the type to have a list of mixed types, as seen in a few other answers. – Chris Rollins Jan 13 '18 at 20:14
39

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()))
{ 
     // ...
}
jason
  • 236,483
  • 35
  • 423
  • 525
27

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.

Tuukka Haapaniemi
  • 1,156
  • 1
  • 11
  • 24
  • `using var outFile = new StreamReader(outputFile.OpenRead()), var expFile = new StreamReader(expectedFile.OpenRead());` also works – Chazt3n Sep 09 '20 at 12:23
  • @Chazt3n No it does not. Implicitly-typed variables cannot have multiple declarators. – Paul Childs May 20 '22 at 00:45
19

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.

Botz3000
  • 39,020
  • 8
  • 103
  • 127
  • 5
    I feel like this would be confusing to a new developer looking at your code. – Zack Jan 06 '15 at 14:40
  • 8
    This can be a bad practice; it has a side-effect that the variables will still exist even after the unmanaged resourced have been freed. According to Microsoft's C# reference, "You can instantiate the resource object and then pass the variable to the using statement, but this is not a best practice. In this case, the object remains in scope after control leaves the using block even though it will probably no longer have access to its unmanaged resources." – Robert Altman Aug 18 '15 at 13:30
  • @RobertAltman You're right, and in real code I would use another approach (probably the one from Gavin H). This is just a less preferrable alternative. – Botz3000 Aug 19 '15 at 07:01
  • You could just move the declarations inside the using with typecasts. Would that be better? – Timothy Blaisdell Dec 11 '19 at 23:48
11

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.

jpierson
  • 16,435
  • 14
  • 105
  • 149
8

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.

Jason Williams
  • 56,972
  • 11
  • 108
  • 137
  • Yeah, checking the file size is a good idea, saves you the time or reading all the bytes. (+1) – TimothyP Aug 25 '09 at 17:50
  • 1
    This doesn't seem to address the question, which is about nesting `using` statements (regardless of what the `using` statements contain). It should be a comment, instead. – TylerH Sep 10 '20 at 15:28
7

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

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

}
Bridge
  • 29,818
  • 9
  • 60
  • 82
codymanix
  • 28,510
  • 21
  • 92
  • 151
7

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?

aquinas
  • 23,318
  • 5
  • 58
  • 81
6

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

yoyoyoyosef
  • 7,000
  • 8
  • 40
  • 39
6

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")

mix3d
  • 4,122
  • 2
  • 25
  • 47
SLaks
  • 868,454
  • 176
  • 1,908
  • 1,964
5

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.

TylerH
  • 20,799
  • 66
  • 75
  • 101
womp
  • 115,835
  • 26
  • 236
  • 269
5

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;  
Charles Bretana
  • 143,358
  • 22
  • 150
  • 216
  • Interesting solution; doing this/even using the 1 set of brackets at lowest level maybe, accomplishes the same goal as stacking them left-justified (cleaner IMO), while addressing the cosmetic nesting desire others mentioned to show any subordination. – galaxis Oct 29 '17 at 14:55
3

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

TylerH
  • 20,799
  • 66
  • 75
  • 101
obelix
  • 986
  • 8
  • 14
3

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.

TimothyP
  • 21,178
  • 26
  • 94
  • 142
  • 2
    Instead of taking a byte array, the method should take a `Stream` object and call the `ReadByte` method until it returns -1. This will save large amounts of memory for large files. – SLaks Aug 25 '09 at 17:53
  • How would you then calculate the crc over all the bytes? – TimothyP Aug 25 '09 at 17:56
  • Oh never mind what I said :p Thnx, I'll change that in my code :p We only use it for data < 1000 bytes so haven't noticed problems yet but will change anyway – TimothyP Aug 25 '09 at 18:00
  • Every time you call `ReadByte` the stream's position advances by one byte. Therefore, if you keep calling it until it returns -1 (EOF), it'll give you every byte in the file. http://msdn.microsoft.com/en-us/library/system.io.stream.readbyte.aspx – SLaks Aug 25 '09 at 18:00
  • 7
    Using a CRC is great if you want to compare multiple files multiple times, but for a single comparison you have to read both files in their entirety to compute the CRCs - If you compare the data in small chunks then you can exit from the comparison as soon as you find a byte that differs. – Jason Williams Aug 25 '09 at 19:51
  • Isn’t a `ushort` a little, eh, “short” of a hash? Depending on the level of assurance needed in determining that files exactly match or where the files being compared come from, I think this method is too likely to find find a hash collision. – binki Feb 19 '14 at 15:55
3

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

hcp
  • 486
  • 1
  • 3
  • 11
  • 1
    Several on one line works if all the things are of the same type. Mixed types have to split over separate using()s. But it doesn't work with var, you have to specify a type (C# 5 specification, p237) – Chris F Carroll Nov 27 '14 at 17:03
3

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;
       }
    }
Rajan Mishra
  • 1,178
  • 2
  • 14
  • 30