tags:

views:

1320

answers:

13

I am working on a project and have to compare two files and see if they match eachother excatly.

My first draft before alot of error checking and validation came up with:

  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?

+24  A: 

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())) {
        ...
    }
ghills
This syntax only works when all the variables are of the same type.
Joel Coehoorn
Yeah I said if they are of the same type, but edited for clarity.
ghills
I did not know that! +1!
TrueWill
+3  A: 

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.

Edit: Too slow on the typing to show consolidated code example. +1 to everyone else.

womp
+9  A: 

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
A: 

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

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

}

codymanix
+42  A: 

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
Wow I learned something today. You can use this approach with different type. +1
Pierre-Alain Vigeant
Yeah that approach is nice when the types are different.
ghills
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
I didn't know this either, cool. Sure does clean up the code
TimothyP
even if your going to trim out the brackets, for sake of convention I recommend you keep the indent on the nested portion
Hardryv
@Hardryv: Visual Studio's auto-format removes it. The idea is to look like a list of variable declarations.
SLaks
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
It seems there is never a shortage of dissenters on any readability issue. I wonder which of them would prefer no nesting if nesting had been recommended...what is the word? Ah yes, contrarian.
Bryan Watts
A: 

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

obelix
+2  A: 

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
You can omit all the brackets in this case...
Charles Bretana
+3  A: 

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
+1  A: 

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

SLaks
+6  A: 

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
Yeah, checking the file size is a good idea, saves you the time or reading all the bytes. (+1)
TimothyP
A: 

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
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
How would you then calculate the crc over all the bytes?
TimothyP
Oh never mind what I said :p Thnx, I'll change that in my code :pWe only use it for data < 1000 bytes so haven't noticed problems yet but will change anyway
TimothyP
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
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
+2  A: 

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
A: 

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