Reputation: 7448
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
Reputation: 241583
When the IDisposable
s 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 IDisposable
s are of the same type:
using (StreamReader outFile = new StreamReader(outputFile.OpenRead()))
using (StreamWriter anotherFile = new StreamReader(anotherFile.OpenRead()))
{
// ...
}
Upvotes: 40
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
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