using and finally statement

[tshad]

Do I still need to close an object in a finally statement after a catch if
it is part of a using statement?

For example:
FileStream fs = null;

try
{
using (FileInfo fInfo = new FileInfo(fromImagePath + "\\" +
(string)dr["originalFileName"]))
{
if (fInfo.Exists)
{
numBytes = fInfo.Length;

// Read the file into the byte array

using (fs = new FileStream(fromImagePath + "\\" +
(string)dr["originalFileName"], FileMode.Open, System.IO.FileAccess.Read))
{
using(BinaryReader br = new BinaryReader(fs))
myByteArray = br.ReadBytes((int)numBytes);
}

// Write out the file into the images folder with new name

using (fs = System.IO.File.OpenWrite(imagePath + imageName))
{
fs.Write(myByteArray, 0, myByteArray.Length);
}
}
}
}
catch (Exception exc)
{
}
finally
{
if (br != null)
br.Close();
if (fs != null)
fs.Close();
}

Do I need to use the finally code or does the "using" automatically close
the objects (fs and br)?

Also, when the you exit the using statement (with or without an error -
which would send you to the catch clause), would the an object that was
defined outside of the using statement be null?

Thanks,

Tom

[Jon Skeet [C# MVP]]

tshad <ts***@dslextreme.comwrote:

Do I still need to close an object in a finally statement after a catch if
it is part of a using statement?

No. The whole point of a using statement is to do that automatically.

I would tend to only declare the FileStream fs in each using statement,
rather than sharing one variable for two uses.

(Note that FileInfo doesn't implement IDisposable, so your code
wouldn't actually compile at the moment.)

Also, when the you exit the using statement (with or without an error -
which would send you to the catch clause), would the an object that was
defined outside of the using statement be null?

No, it would have the same value, it's just the object would have been
disposed and therefore not terribly useful. That's one reason to
declare the variable in the using statement itself - it stops you from
trying to use an object which has already been disposed.

--
Jon Skeet - <sk***@pobox.com>
Web site: http://www.pobox.com/~skeet
Blog: http://www.msmvps.com/jon.skeet
C# in Depth: http://csharpindepth.com

[tshad]

"Jon Skeet [C# MVP]" <sk***@pobox.comwrote in message
news:MP*********************@msnews.microsoft.com. ..

tshad <ts***@dslextreme.comwrote:

>Do I still need to close an object in a finally statement after a catch
if
it is part of a using statement?

No. The whole point of a using statement is to do that automatically.

I would tend to only declare the FileStream fs in each using statement,
rather than sharing one variable for two uses.

(Note that FileInfo doesn't implement IDisposable, so your code
wouldn't actually compile at the moment.)

>Also, when the you exit the using statement (with or without an error -
which would send you to the catch clause), would the an object that was
defined outside of the using statement be null?

No, it would have the same value, it's just the object would have been
disposed and therefore not terribly useful. That's one reason to
declare the variable in the using statement itself - it stops you from
trying to use an object which has already been disposed.

Makes sense.

In the following:

using (fs = new FileStream(fromImagePath + "\\" +
(string)dr["originalFileName"], FileMode.Open, System.IO.FileAccess.Read))
{
using(BinaryReader br = new BinaryReader(fs))
myByteArray = br.ReadBytes((int)numBytes);
}

Is putting the BinaryReader in the using statement a little overkill?

Thanks,

Tom

--
Jon Skeet - <sk***@pobox.com>
Web site: http://www.pobox.com/~skeet
Blog: http://www.msmvps.com/jon.skeet
C# in Depth: http://csharpindepth.com

[Jon Skeet [C# MVP]]

tshad <ts***@dslextreme.comwrote:

Makes sense.

In the following:

using (fs = new FileStream(fromImagePath + "\\" +
(string)dr["originalFileName"], FileMode.Open, System.IO.FileAccess.Read))
{
using(BinaryReader br = new BinaryReader(fs))
myByteArray = br.ReadBytes((int)numBytes);
}

Is putting the BinaryReader in the using statement a little overkill?

Yes, but I tend to do it anyway - it's better to get into the habit of
always disposing of things, and there can be wrapping cases like this
where it *does* make a difference (because closing the inner resource
may flush pending writes to the outer resource).

--
Jon Skeet - <sk***@pobox.com>
Web site: http://www.pobox.com/~skeet
Blog: http://www.msmvps.com/jon.skeet
C# in Depth: http://csharpindepth.com

[tshad]

"Jon Skeet [C# MVP]" <sk***@pobox.comwrote in message
news:MP*********************@msnews.microsoft.com. ..

tshad <ts***@dslextreme.comwrote:

>Do I still need to close an object in a finally statement after a catch
if
it is part of a using statement?

No. The whole point of a using statement is to do that automatically.

I would tend to only declare the FileStream fs in each using statement,
rather than sharing one variable for two uses.

(Note that FileInfo doesn't implement IDisposable, so your code
wouldn't actually compile at the moment.)

>Also, when the you exit the using statement (with or without an error -
which would send you to the catch clause), would the an object that was
defined outside of the using statement be null?

No, it would have the same value, it's just the object would have been
disposed and therefore not terribly useful. That's one reason to
declare the variable in the using statement itself - it stops you from
trying to use an object which has already been disposed.

So there would be no reason to do this:

Font font2 = new Font("Arial", 10.0f);
using (font2)
{
// use font2
}
or

FileStream fs = null;
using (fs = new FileStream(originalFileName, FileMode.Open,
System.IO.FileAccess.Read))
{
using(BinaryReader br = new BinaryReader(fs))
myByteArray = br.ReadBytes((int)numBytes);
}

since fs would not be closed (or would it?).

Thanks,

Tom

--
Jon Skeet - <sk***@pobox.com>
Web site: http://www.pobox.com/~skeet
Blog: http://www.msmvps.com/jon.skeet
C# in Depth: http://csharpindepth.com

[tshad]

"Jon Skeet [C# MVP]" <sk***@pobox.comwrote in message
news:MP*********************@msnews.microsoft.com. ..

tshad <ts***@dslextreme.comwrote:

>Do I still need to close an object in a finally statement after a catch
if
it is part of a using statement?

No. The whole point of a using statement is to do that automatically.

I would tend to only declare the FileStream fs in each using statement,
rather than sharing one variable for two uses.

So you wouldn't do the following?:

FileStream fs = null;
try
{
fs = System.IO.File.OpenWrite(imagePath + imageName);
fs.Write(myByteArray, 0, myByteArray.Length);
fs.Close();

if (dr["thumbnailImage"].ToString() != "")
{
myByteArray2 =
Convert.FromBase64String(dr["thumbnailImage"].ToString());
if (myByteArray2.Length 0)
{
thumbnailName = appraisalID.ToString() + "_" +
(imageKtr).ToString() + "_Thumb" + "." + imageExtension;
fs = System.IO.File.OpenWrite(imagePath + thumbnailName);
fs.Write(myByteArray2, 0, myByteArray2.Length);
fs.Close();
}
}
}
catch(Exception exc)
{
}
finally
{
if (fs=null)
fs.Close();
}

There could also be 2 or 3 other fs assignments for moving some other files.

Also, I am not using "new" for any of these - does that make a difference?

Thanks,

Tom

>
--
Jon Skeet - <sk***@pobox.com>
Web site: http://www.pobox.com/~skeet
Blog: http://www.msmvps.com/jon.skeet
C# in Depth: http://csharpindepth.com

[Peter Duniho]

On Sun, 01 Jun 2008 00:22:56 -0700, tshad <ts***@dslextreme.comwrote:

So there would be no reason to do this:

Font font2 = new Font("Arial", 10.0f);
using (font2)
{
// use font2
}
or

FileStream fs = null;
using (fs = new FileStream(originalFileName, FileMode.Open,
System.IO.FileAccess.Read))
{
using(BinaryReader br = new BinaryReader(fs))
myByteArray = br.ReadBytes((int)numBytes);
}

since fs would not be closed (or would it?).

In each example, the resource will be _disposed_ (not necessarily
closed). The main issue is that the variable would still be in scope
after you disposed the object, which in most cases would be pointless and
could lead to incorrect code.

For many objects, Close() and Dispose() do the exact same thing. But you
shouldn't rely on that as a general rule. The "using" statement disposes
things, not closes things. If the docs promise you that Dispose() and
Close() are identical, it's a reasonable shortcut, but otherwise you
should get into the habit of doing both.

Pete

[Peter Duniho]

On Sun, 01 Jun 2008 00:45:54 -0700, tshad <ts***@dslextreme.comwrote:

So you wouldn't do the following?:

I can't speak for Jon. But I'd for sure make sure it at least compiled
(you have a syntax error in your final "if" statement). :)

Seriously though, especially assuming you've got the try/catch there
anyway, I don't see anything wrong with that particular pattern per se.

However, I personally would more likely have a complete "using" with
variable declaration around each place I'm actually using a FileStream.
Is it necessary to write it that way? No. But it is IMHO slightly
cleaner looking, and helps avoid code that might mistakenly use the "fs"
variable when it's not currently representing something useful.

[...]
Also, I am not using "new" for any of these - does that make a
difference?

A difference for what? How you call Dispose() or otherwise use the
instance? No...whether you call "new" yourself or some other code does it
on your behalf, if you've got an instance, you need to clean it up.

Pete

[tshad]

Peter Duniho wrote:

On Sun, 01 Jun 2008 00:22:56 -0700, tshad <ts***@dslextreme.com>
wrote:

>So there would be no reason to do this:

Font font2 = new Font("Arial", 10.0f);
using (font2)
{
// use font2
}
or

FileStream fs = null;
using (fs = new FileStream(originalFileName, FileMode.Open,
System.IO.FileAccess.Read))
{
using(BinaryReader br = new BinaryReader(fs))
myByteArray = br.ReadBytes((int)numBytes);
}

since fs would not be closed (or would it?).

In each example, the resource will be _disposed_ (not necessarily
closed). The main issue is that the variable would still be in scope
after you disposed the object, which in most cases would be pointless
and could lead to incorrect code.

For many objects, Close() and Dispose() do the exact same thing. But
you shouldn't rely on that as a general rule. The "using" statement
disposes things, not closes things. If the docs promise you that
Dispose() and Close() are identical, it's a reasonable shortcut, but
otherwise you should get into the habit of doing both.

But if I change my code to:

using (File Stream fs = new FileStream(originalFileName, FileMode.Open,
System.IO.FileAccess.Read))
{
using(BinaryReader br = new BinaryReader(fs))
myByteArray = br.ReadBytes((int)numBytes);
}

Would I still do the Close() on both objects? Something like:

using (File Stream fs = new FileStream(originalFileName, FileMode.Open,
System.IO.FileAccess.Read))
{
using(BinaryReader br = new BinaryReader(fs))
myByteArray = br.ReadBytes((int)numBytes);
fs.Close();
br.Close();
}

I assume that you wouldn't use the "Closes" in a finally clause outside of
the using statement since the fs and br variables would be out of scope.

Thanks,

Tom

>
Pete

[tshad]

Peter Duniho wrote:

On Sun, 01 Jun 2008 00:45:54 -0700, tshad <ts***@dslextreme.com>
wrote:

>So you wouldn't do the following?:

I can't speak for Jon. But I'd for sure make sure it at least
compiled (you have a syntax error in your final "if" statement). :)

Actually, it does compile. Which error?

>
Seriously though, especially assuming you've got the try/catch there
anyway, I don't see anything wrong with that particular pattern per
se.
However, I personally would more likely have a complete "using" with
variable declaration around each place I'm actually using a
FileStream. Is it necessary to write it that way? No. But it is
IMHO slightly cleaner looking, and helps avoid code that might
mistakenly use the "fs" variable when it's not currently representing
something useful.

I guess it is cleaner looking.

So you would write it like (without the trys)?:

using(fs = System.IO.File.OpenWrite(imagePath + imageName))
fs.Write(myByteArray, 0, myByteArray.Length);

if (dr["thumbnailImage"].ToString() != "")
{
myByteArray2 =
Convert.FromBase64String(dr["thumbnailImage"].ToString());
if (myByteArray2.Length 0)
{
thumbnailName = appraisalID.ToString() + "_" +
(imageKtr).ToString() + "_Thumb" + "." + imageExtension;

using(fs = System.IO.File.OpenWrite(imagePath + thumbnailName))
fs.Write(myByteArray2, 0, myByteArray2.Length);
}
}

Thanks,

Tom

>[...]

Pete

[Jon Skeet [C# MVP]]

tshad <tf*@dslextreme.comwrote:

For many objects, Close() and Dispose() do the exact same thing. But
you shouldn't rely on that as a general rule. The "using" statement
disposes things, not closes things. If the docs promise you that
Dispose() and Close() are identical, it's a reasonable shortcut, but
otherwise you should get into the habit of doing both.

But if I change my code to:

using (File Stream fs = new FileStream(originalFileName, FileMode.Open,
System.IO.FileAccess.Read))
{
using(BinaryReader br = new BinaryReader(fs))
myByteArray = br.ReadBytes((int)numBytes);
}

Would I still do the Close() on both objects? Something like:

No, because FileStream.Dispose and BinaryReader.Dispose are equivalent
to closing them.

--
Jon Skeet - <sk***@pobox.com>
Web site: http://www.pobox.com/~skeet
Blog: http://www.msmvps.com/jon.skeet
C# in Depth: http://csharpindepth.com

[Jon Skeet [C# MVP]]

tshad <tf*@dslextreme.comwrote:

Peter Duniho wrote:

On Sun, 01 Jun 2008 00:45:54 -0700, tshad <ts***@dslextreme.com>
wrote:

So you wouldn't do the following?:

I can't speak for Jon. But I'd for sure make sure it at least
compiled (you have a syntax error in your final "if" statement). :)

Actually, it does compile.

No, it doesn't.

Which error?

if (fs=null)

should be

if (fs!=null)

Seriously though, especially assuming you've got the try/catch there
anyway, I don't see anything wrong with that particular pattern per
se.
However, I personally would more likely have a complete "using" with
variable declaration around each place I'm actually using a
FileStream. Is it necessary to write it that way? No. But it is
IMHO slightly cleaner looking, and helps avoid code that might
mistakenly use the "fs" variable when it's not currently representing
something useful.

I guess it is cleaner looking.

So you would write it like (without the trys)?:

using(fs = System.IO.File.OpenWrite(imagePath + imageName))
fs.Write(myByteArray, 0, myByteArray.Length);

if (dr["thumbnailImage"].ToString() != "")
{
myByteArray2 =
Convert.FromBase64String(dr["thumbnailImage"].ToString());
if (myByteArray2.Length 0)
{
thumbnailName = appraisalID.ToString() + "_" +
(imageKtr).ToString() + "_Thumb" + "." + imageExtension;

using(fs = System.IO.File.OpenWrite(imagePath + thumbnailName))
fs.Write(myByteArray2, 0, myByteArray2.Length);
}
}

No, I'd write it as:

using (FileStream fs = File.OpenWrite(imagePath + imageName))
{
fs.Write(myByteArray, 0, myByteArray.Length);
}

....

There's no need to reuse the fs variable - just declare it once per
using statement. Why do you feel the need to use one variable for two
different pieces of code?

(In fact, I'd use File.WriteAllBytes to make the code simpler in the
first place, but...)

--
Jon Skeet - <sk***@pobox.com>
Web site: http://www.pobox.com/~skeet
Blog: http://www.msmvps.com/jon.skeet
C# in Depth: http://csharpindepth.com

[tshad]

"Jon Skeet [C# MVP]" <sk***@pobox.comwrote in message
news:MP*********************@msnews.microsoft.com. ..

tshad <tf*@dslextreme.comwrote:

>Peter Duniho wrote:

On Sun, 01 Jun 2008 00:45:54 -0700, tshad <ts***@dslextreme.com>
wrote:
So you wouldn't do the following?:

I can't speak for Jon. But I'd for sure make sure it at least
compiled (you have a syntax error in your final "if" statement). :)

Actually, it does compile.

No, it doesn't.

>Which error?

if (fs=null)

should be

if (fs!=null)

You're right it wouldn't have compiled. Didn't even see that one.

It compiles because the you have is how I actually have it in my code
(fs!=null) . I typed it in just for the example and obviously mistyped it.

>

Seriously though, especially assuming you've got the try/catch there
anyway, I don't see anything wrong with that particular pattern per
se.
However, I personally would more likely have a complete "using" with
variable declaration around each place I'm actually using a
FileStream. Is it necessary to write it that way? No. But it is
IMHO slightly cleaner looking, and helps avoid code that might
mistakenly use the "fs" variable when it's not currently representing
something useful.

I guess it is cleaner looking.

So you would write it like (without the trys)?:

using(fs = System.IO.File.OpenWrite(imagePath + imageName))
fs.Write(myByteArray, 0, myByteArray.Length);

if (dr["thumbnailImage"].ToString() != "")
{
myByteArray2 =
Convert.FromBase64String(dr["thumbnailImage"].ToString());
if (myByteArray2.Length 0)
{
thumbnailName = appraisalID.ToString() + "_" +
(imageKtr).ToString() + "_Thumb" + "." + imageExtension;

using(fs = System.IO.File.OpenWrite(imagePath +
thumbnailName))
fs.Write(myByteArray2, 0, myByteArray2.Length);
}
}

No, I'd write it as:

using (FileStream fs = File.OpenWrite(imagePath + imageName))
{
fs.Write(myByteArray, 0, myByteArray.Length);
}

...

There's no need to reuse the fs variable - just declare it once per
using statement. Why do you feel the need to use one variable for two
different pieces of code?

You're right.

I guess I just felt that if I am going to use the FileStream in 2 or 3
different places that it was better to use the same variable instead of the
creation of the object 3 times. Seemed like the overhead (which is neglible
anyway) would make for better code. Probably wrong there:)

(In fact, I'd use File.WriteAllBytes to make the code simpler in the
first place, but...)

Actually, I would also. I just didn't know it was there. I have always
used fs.Write. Is there a downside to using WriteAllBytes?

Thanks,

Tom

>
--
Jon Skeet - <sk***@pobox.com>
Web site: http://www.pobox.com/~skeet
Blog: http://www.msmvps.com/jon.skeet
C# in Depth: http://csharpindepth.com

[Jon Skeet [C# MVP]]

tshad <ts***@dslextreme.comwrote:

You're right it wouldn't have compiled. Didn't even see that one.

It compiles because the you have is how I actually have it in my code
(fs!=null) . I typed it in just for the example and obviously mistyped it.

That's why it's always better to cut and paste than retype.

There's no need to reuse the fs variable - just declare it once per
using statement. Why do you feel the need to use one variable for two
different pieces of code?

You're right.

I guess I just felt that if I am going to use the FileStream in 2 or 3
different places that it was better to use the same variable instead of the
creation of the object 3 times. Seemed like the overhead (which is neglible
anyway) would make for better code. Probably wrong there:)

The compiler can deal with the performance overhead if it chooses to.
It's almost certainly negligible, if it's even present. The
*readability* overhead of reusing the same variable *isn't* negligible,
however. You've conceptually got two different variables, so make them
two different variables in reality. Always go for readability over
performance until you've proven that the performance difference is
worth the readability difference :)

(In fact, I'd use File.WriteAllBytes to make the code simpler in the
first place, but...)

Actually, I would also. I just didn't know it was there. I have always
used fs.Write. Is there a downside to using WriteAllBytes?

It's not easily transferrable if you want to write to something other
than a file, but that's about it as far as I'm aware.

--
Jon Skeet - <sk***@pobox.com>
Web site: http://www.pobox.com/~skeet
Blog: http://www.msmvps.com/jon.skeet
C# in Depth: http://csharpindepth.com

[Peter Duniho]

On Sun, 01 Jun 2008 01:26:12 -0700, tshad <tf*@dslextreme.comwrote:

[...snip...]

I would reply, but Jon pretty much covered everything. :)

[tshad]

"Jon Skeet [C# MVP]" <sk***@pobox.comwrote in message
news:MP*********************@msnews.microsoft.com. ..

tshad <ts***@dslextreme.comwrote:

>You're right it wouldn't have compiled. Didn't even see that one.

It compiles because the you have is how I actually have it in my code
(fs!=null) . I typed it in just for the example and obviously mistyped
it.

That's why it's always better to cut and paste than retype.

True.

>

There's no need to reuse the fs variable - just declare it once per
using statement. Why do you feel the need to use one variable for two
different pieces of code?

You're right.

I guess I just felt that if I am going to use the FileStream in 2 or 3
different places that it was better to use the same variable instead of
the
creation of the object 3 times. Seemed like the overhead (which is
neglible
anyway) would make for better code. Probably wrong there:)

The compiler can deal with the performance overhead if it chooses to.
It's almost certainly negligible, if it's even present. The
*readability* overhead of reusing the same variable *isn't* negligible,
however. You've conceptually got two different variables, so make them
two different variables in reality. Always go for readability over
performance until you've proven that the performance difference is
worth the readability difference :)

Agreed.

(In fact, I'd use File.WriteAllBytes to make the code simpler in the
first place, but...)

Actually, I would also. I just didn't know it was there. I have always
used fs.Write. Is there a downside to using WriteAllBytes?

It's not easily transferrable if you want to write to something other
than a file, but that's about it as far as I'm aware.

If it's there - I may as well use it.

Thanks,

Tom

>
--
Jon Skeet - <sk***@pobox.com>
Web site: http://www.pobox.com/~skeet
Blog: http://www.msmvps.com/jon.skeet
C# in Depth: http://csharpindepth.com