Today, I wanted to perform an operation with a file so I came up with this code
class Test1
{
Test1()
{
using (var fileStream = new FileStream("c:\\test.txt", FileMode.Open))
{
//just use this filestream in a using Statement and release it after use.
}
}
}
But on code review, I was asked to implement IDisposable interface and Finalizer methods
class Test : IDisposable
{
Test()
{
//using some un managed resources like files or database connections.
}
~Test()
{
//since .NET garbage collector, does not call Dispose method, i call in Finalize method since .net garbage collector calls this
}
public void Dispose()
{
//release my files or database connections
}
}
But, my question is why should I ?
Although I cannot justify my methodology according to me, why should we use IDisposable when using statement can itself release resources)
Any specific advantages or am is missing something here?
in your example using statement is right, because you use resources only in scope of your method. For example:
Test1()
{
using (FileStream fs = new FileStream("c:\\test.txt", FileMode.Open))
{
byte[] bufer = new byte[256];
fs.Read(bufer, 0, 256);
}
}
but if the resources are used outside of one method, then you should create Dispose method. This code is wrong:
class Test1
{
FileStream fs;
Test1()
{
using (var fileStream = new FileStream("c:\\test.txt", FileMode.Open))
{
fs = fileStream;
}
}
public SomeMethod()
{
byte[] bufer = new byte[256];
fs.Read(bufer, 0, 256);
}
}
The rigth thing to do is implement IDisposable
to be sure, that the file will be released after it is used.
class Test1 : IDisposable
{
FileStream fs;
Test1()
{
fs = new FileStream("c:\\test.txt", FileMode.Open);
}
public SomeMethod()
{
byte[] bufer = new byte[256];
fs.Read(bufer, 0, 256);
}
public void Dispose()
{
if(fs != null)
{
fs.Dispose();
fs = null;
}
}
}
A little note first, since you seem a bit confused about how using
and IDisposable
interact with each other: The reason why you're able to say using (FileStream fileStream = Whatever()) { ... }
is precisely because the FileStream
class implements IDisposable
. What your coworkers have suggested is that you implement IDisposable
on your class so that you'll be able to say using (Test test = new Test()) { ... }
.
For what it's worth, I think the way you wrote the code initially is strongly preferable to the suggested change, unless there's some compelling reason why you might want to keep the FileStream
open for the entire lifetime of a Test1
instance. One reason why this might be the case is that the file might be subject to change from some other source after the constructor of Test1
has been called, in which case you'll be stuck with an older copy of the data. Another reason for keeping the FileStream
open could be if you specifically want to lock the file from being written to from elsewhere while a Test1
object is alive.
In general, it's good practice to release resources as soon as possible, which your original code seems to do. One thing I'm a little skeptical of is that the work is done in your constructor instead of in some method that's explicitly called from the outside (explanation: http://misko.hevery.com/code-reviewers-guide/flaw-constructor-does-real-work/). But this is a different matter altogether, and independent of the question of whether to make your class implement IDisposable
.
If you love us? You can donate to us via Paypal or buy me a coffee so we can maintain and grow! Thank you!
Donate Us With