BinaryWriter/Reader question

[Filip Strugar]

Considering that the BinaryWriter/BinaryReader object closes the underlaying
stream upon being gc collected, is the following code correct, and if it is
what is the reason preventing BinaryWriter object garbage collection after
the WriteSomething method is executed?

----

using System;
using System.IO;

namespace Test
{
class Test
{
static void Main(string[] args)
{
using (Stream output = (Stream)File.OpenWrite("test.txt"))
{
WriteSomething(output);
WriteSomethingElse(output);
}
}

static void WriteSomething(Stream outputStream)
{
BinaryWriter writer = new BinaryWriter(outputStream);
writer.Write("Something");
}

static void WriteSomethingElse(Stream outputStream)
{
BinaryWriter writer = new BinaryWriter(outputStream);
writer.Write("SomethingElse");
}
}
}

---

[pdav...@hotmail.com]

Your code looks fine. Nothing prevents the garbage collector from collecting
the BinaryWriter in WriteSomething(). Why do you care? It's out of scope at
that point.

It has no Finalize() method, so nothing is going to happen when it gets
collected anyway.

Pete

"Filip Strugar" <fili@sezampro_dot_yu> wrote in message
news:eHHolvOR...@TK2MSFTNGP10.phx.gbl...

[Filip Strugar]

Will it get disposed?
When BinaryWriter/Reader gets disposed, it closes the underlying stream, so
no further writing (reading) on it is possible!

<pdav...@hotmail.com> wrote in message
news:19e230b4068fc9df...@news.meganetnews.com...

[Jon Skeet [C# MVP]]

Filip Strugar <fili@sezampro_dot_yu> wrote:
> Will it get disposed?
> When BinaryWriter/Reader gets disposed, it closes the underlying stream, so
> no further writing (reading) on it is possible!

It would only be Disposed if you *told* it to be Disposed, either with
an explicit call or with a using statement (or another method call
which called it).

--
Jon Skeet - <sk...@pobox.com>
http://www.pobox.com/~skeet
If replying to the group, please do not mail me too

[pdav...@hotmail.com]

As Jon pointed out, it's only disposed if you explicitly call the Dispose
method. But in the case of BinaryWriter, Dispose() is a protected member, so
you'd only call it from a derived class anyway. And even then, if you call
Dispose(false), the underlying stream won't be closed.

The implementation of the Dispose() method in BinaryWriter simply checks to
see if Disposing() is true. If it is, then it closes the underlying stream.
Otherwise it does nothing.

Pete

"Filip Strugar" <fili@sezampro_dot_yu> wrote in message

news:eSFlO4QR...@TK2MSFTNGP10.phx.gbl...

[Jon Skeet [C# MVP]]

<pdav...@hotmail.com> wrote:
> As Jon pointed out, it's only disposed if you explicitly call the Dispose
> method. But in the case of BinaryWriter, Dispose() is a protected member, so
> you'd only call it from a derived class anyway.

No, Dispose(bool) is a protected member, but just Dispose() isn't - and
that's what you'd usually call. It calls Dispose(true).

[pdav...@hotmail.com]

BinaryWriter only has the Dispose(bool). It does not have a Dispose() method
without parameters.

Pete

"Jon Skeet [C# MVP]" <sk...@pobox.com> wrote in message
news:MPG.1b22ac4c9...@msnews.microsoft.com...

[pdav...@hotmail.com]

Hmm, this is strange.

BinaryWriter is from IDisposable, but I can't find the implementation of the
Dispose() method.

What's that all about? There's no Dispose() method documented. Reflector
shows no implementation of Dispose(). It doesn't show up in intellisense...
What's the deal?

How can it derive from IDisposable and not implement Dispose()?

Pete

<pdav...@hotmail.com> wrote in message
news:8536a5ef02457ba8...@news.meganetnews.com...

[Jon Skeet [C# MVP]]

<pdav...@hotmail.com> wrote:
> BinaryWriter only has the Dispose(bool). It does not have a Dispose() method
> without parameters.

Yes it does. It has to - it implements IDisposable. However, it
implements it using explicit interface implementation, so you have to
cast to IDisposable (or use a using statement):

BinaryWriter foo = new BinaryWriter(...);
...
((IDisposable)foo).Dispose();

or

using (BinaryWriter foo = new BinaryWriter(...))
{
...

[pdav...@hotmail.com]

Okay, I see how it works now. I didn't see the Dispose method because I was
looking at the methods in alphabetical order and it falls under
System.IDisposable.Dispose().

Thanks for clearing that up.

Now, what seems strange is the implementation is:

private void System.IDisposable.Dispose()
{
this.Dispose(true);
}

If it's declared private, how can I call it (which I can, I verified your
typecasting code works).

Pete

P.S. thanks for the education here.

"Jon Skeet [C# MVP]" <sk...@pobox.com> wrote in message

news:MPG.1b22b2cb1...@msnews.microsoft.com...

[Jon Skeet [C# MVP]]

<pdav...@hotmail.com> wrote:
> Now, what seems strange is the implementation is:
>
> private void System.IDisposable.Dispose()
> {
> this.Dispose(true);
> }
>
> If it's declared private, how can I call it (which I can, I verified your
> typecasting code works).
>
> Pete
>
> P.S. thanks for the education here.

Explicit interface implementation is strange like that. It's sort of
private, sort of public. See

http://www.jaggersoft.com/csharp_standard/20.4.1.htm

for more information.

[pdav...@hotmail.com]

Thanks. That was really informative. I can see how that would be useful from
time to time.

I think we can assume that it was implemented this way in BinaryWriter
because of the first justification (Paragraph 5)

"This is particularly useful when a class or struct implements an internal
interface that is of no interest to a consumer of that class or struct. "

The other justifcation being to disambiguate interface members with the same
signature, which wouldn't be the case here.

The ability to make it publicly accessible would be for justification #2 it
seems, so in this particular case, can I assume that calling Dispose()
directly would be a bad idea?

Thanks again. This has been really englightening, albeit in a somewhat
obscure area of C#.

Pete

"Jon Skeet [C# MVP]" <sk...@pobox.com> wrote in message

news:MPG.1b22d2d63...@msnews.microsoft.com...

[Jon Skeet [C# MVP]]

<pdav...@hotmail.com> wrote:
> Thanks. That was really informative. I can see how that would be useful from
> time to time.

Personally I think it's much more rarely useful than MS seems to...

> I think we can assume that it was implemented this way in BinaryWriter
> because of the first justification (Paragraph 5)
>
> "This is particularly useful when a class or struct implements an internal
> interface that is of no interest to a consumer of that class or struct. "

I'm not sure - it seems as natural to me to want to Dispose of a
BinaryWriter as (say) a StreamWriter. They're both just wrappers round
streams which add value.

> The other justifcation being to disambiguate interface members with the same
> signature, which wouldn't be the case here.
>
> The ability to make it publicly accessible would be for justification #2 it
> seems, so in this particular case, can I assume that calling Dispose()
> directly would be a bad idea?

Not at all - it's something I'd do as a matter of course:

using (Stream stream = ...)
{
using (BinaryWriter writer = ...)
{
...
}
}

I *always* dispose of *everything* which implements IDisposable and of
which I have direct knowledge of the useful lifetime.

> Thanks again. This has been really englightening, albeit in a somewhat
> obscure area of C#.

Glad it's helped :)

[pdav...@hotmail.com]

> > The ability to make it publicly accessible would be for justification #2
it
> > seems, so in this particular case, can I assume that calling Dispose()
> > directly would be a bad idea?
>
> Not at all - it's something I'd do as a matter of course:
>

Yes, but in the example originally offered by Flip, where the person created
the stream in one method, then called two separate methods that created
BinaryWriters against that stream, if the person were to call Dispose() on
the BinaryWriter in the first method, it would have closed the stream prior
to the second method being called.

Obviously, in this case, calling Dispose() would cause the program to fail
in the second method call.

I guess in this case, it helps to know what's going on underneath, but I
would infer that the fact that Dispose() is implemented this way and
furthermore, that Dispose() isn't even documented for the BinaryWriter, that
calling Dispose() on the BinaryWriter is probably not always a good idea.

Besides, all it does is close the Stream which is going to happen anyway.
But again, without knowing the underlying implementation and without it
being documented, this wouldn't be known (except for the fact that
unobfuscated code is so easy to reverse engineer in .NET).

Actually, I have never used "using" except for accessing namespaces myself.
I don't doubt its usefulness nor its readability. Simply haven't adopted it
as part of my programming style.... yet....

Pete

Pete

[Jon Skeet [C# MVP]]

<pdav...@hotmail.com> wrote:
> > > The ability to make it publicly accessible would be for justification #2
> it
> > > seems, so in this particular case, can I assume that calling Dispose()
> > > directly would be a bad idea?
> >
> > Not at all - it's something I'd do as a matter of course:

> Yes, but in the example originally offered by Flip, where the person created
> the stream in one method, then called two separate methods that created
> BinaryWriters against that stream, if the person were to call Dispose() on
> the BinaryWriter in the first method, it would have closed the stream prior
> to the second method being called.

Yup.

> Obviously, in this case, calling Dispose() would cause the program to fail
> in the second method call.

Indeed. The same would be true for StreamWriter, too. I've always
thought it would be nice to have some way of constructing those objects
so that the stream could be "detached" from the wrapper. That would
make it very clear what's involved.

> I guess in this case, it helps to know what's going on underneath, but I
> would infer that the fact that Dispose() is implemented this way and
> furthermore, that Dispose() isn't even documented for the BinaryWriter

> that calling Dispose() on the BinaryWriter is probably not always a good idea.

It's not *always* a good idea (as in this case) but I believe that when
you know the lifetime of the stream as well, it's a good idea, just
going along with the idea that it implements IDisposable for a reason.

BinaryWriter.IDisposable.Dispose actually *is* documented, but as "This
member supports the .NET Framework infrastructure and is not intended
to be used directly from your code." I think this is a *big* mistake,
and breaks the whole idea of interfaces.

> Besides, all it does is close the Stream which is going to happen anyway.

It's going to happen at *some* stage, sure. I just think that getting
into the habit of disposing of objects which implement IDisposable and
thinking about the consequences of doing so is a good habit to get
into.

> But again, without knowing the underlying implementation and without it
> being documented, this wouldn't be known (except for the fact that
> unobfuscated code is so easy to reverse engineer in .NET).
>
> Actually, I have never used "using" except for accessing namespaces
> myself. I don't doubt its usefulness nor its readability. Simply
> haven't adopted it as part of my programming style.... yet....

I would seriously consider doing so in the *very* near future. Are you
currently closing streams etc in finally blocks? If not, you're risking
problems if an exception is thrown. If you are, then your code would
become more readable immediately by using the using statement.

[pdav...@hotmail.com]

"Jon Skeet [C# MVP]" <sk...@pobox.com> wrote in message

news:MPG.1b232183e...@msnews.microsoft.com...
> <pdav...@hotmail.com> wrote:
[snip (trying to keep these messages manageable)]

> > I guess in this case, it helps to know what's going on underneath, but I
> > would infer that the fact that Dispose() is implemented this way and
> > furthermore, that Dispose() isn't even documented for the BinaryWriter
> > that calling Dispose() on the BinaryWriter is probably not always a good
idea.
>
> It's not *always* a good idea (as in this case) but I believe that when
> you know the lifetime of the stream as well, it's a good idea, just
> going along with the idea that it implements IDisposable for a reason.
>
> BinaryWriter.IDisposable.Dispose actually *is* documented, but as "This
> member supports the .NET Framework infrastructure and is not intended
> to be used directly from your code." I think this is a *big* mistake,
> and breaks the whole idea of interfaces.
>

Yes, but wouldn't the idea that they've chosen to implement it as an
explicit interface hint that it shouldn't be used by user's code? So isn't
the documentation simply confirming that?

BTW, you must be using different documentation. My MSDN (Jan 2004) only
shows the protected Dispose(bool).

> > Besides, all it does is close the Stream which is going to happen
anyway.
>
> It's going to happen at *some* stage, sure. I just think that getting
> into the habit of disposing of objects which implement IDisposable and
> thinking about the consequences of doing so is a good habit to get
> into.
>

Well, I think all this explicit implementation stuff simply adds a layer of
complexity that is more annoying than helpful. I'm all for simplifying as
much as possible. I figure if you don't want people to call a method, then
you shouldn't implement it in a callable fashion (i.e. make it private and
don't do it as part of the IDisposable interface to begin with).

I mean, you're right, if they implement IDisposable, then I should be
expected to call Dispose(), but they're kind of saying not to do it. I mean,
that's kind of stupid, in my mind. And in this case, it could have
unintended consequences for the user. It screams out bad design somewhere.

> > But again, without knowing the underlying implementation and without it
> > being documented, this wouldn't be known (except for the fact that
> > unobfuscated code is so easy to reverse engineer in .NET).
> >
> > Actually, I have never used "using" except for accessing namespaces
> > myself. I don't doubt its usefulness nor its readability. Simply
> > haven't adopted it as part of my programming style.... yet....
>
> I would seriously consider doing so in the *very* near future. Are you
> currently closing streams etc in finally blocks? If not, you're risking
> problems if an exception is thrown. If you are, then your code would
> become more readable immediately by using the using statement.
>

I'm big on try/finally I don't know why. It adds an extra layer of
indentation and yeah, it's kind of ugly. I just kind of got started that
way (probably some book I read early on in my C# programming) and have been
doing it ever since. What can I say? Habits are hard to break.

Pete

[Justin Rogers]

> Well, I think all this explicit implementation stuff simply adds a layer of
> complexity that is more annoying than helpful. I'm all for simplifying as
> much as possible. I figure if you don't want people to call a method, then
> you shouldn't implement it in a callable fashion (i.e. make it private and
> don't do it as part of the IDisposable interface to begin with).
>
> I mean, you're right, if they implement IDisposable, then I should be
> expected to call Dispose(), but they're kind of saying not to do it. I mean,
> that's kind of stupid, in my mind. And in this case, it could have
> unintended consequences for the user. It screams out bad design somewhere.

They are using explicit binding so they can maintain functionality with C# to
avoid a common programming mistake of not closing an underlying object.
Explicit binding is chosen in this case so that they can rename the API that
you actually should be calling to shut-down a BinaryWriter and that is the
Close method.

You'll notice that both the public virtual Close and protected virtual
Dispose(bool)
share something in common. They are both virtual, and you could easily
write your own wrappers that disabled the shut-down of the underlying
stream. If this is the case, why implement IDisposable in the first case, well
we
are right back to C# and avoiding common programming mistakes. The Mort
user needs stuff cleaned up because they do things like lose the original stream
in calls such as:

new BinaryWriter(File.OpenText(...));

Simple as that. Confuses seasoned programmers, might not be the best way to
go, but there are more Mort's out there than the 2 E's.

> I'm big on try/finally I don't know why. It adds an extra layer of
> indentation and yeah, it's kind of ugly. I just kind of got started that
> way (probably some book I read early on in my C# programming) and have been
> doing it ever since. What can I say? Habits are hard to break.

The reason for the using statement is to gain the extra changes that the keyword
may or may not allow in the future. If you read the C# Team/FAQ blog then
you'll
realize there are some performance changes for the using statement that you
might
want to take advantage of, and some logic that they automatically build in based
on
compile time checks. I'll add that you can easily do this yourself in your own
optimized try/finally, but it actually saves you a bunch of work in the long run
and
makes your code more compatible with types that you don't own and ensures that
your code will easily compile on future versions of .NET.

--
Justin Rogers
DigiTec Web Consultants, LLC.
Blog: http://weblogs.asp.net/justin_rogers

[Jon Skeet [C# MVP]]

<pdav...@hotmail.com> wrote:
> > BinaryWriter.IDisposable.Dispose actually *is* documented, but as "This
> > member supports the .NET Framework infrastructure and is not intended
> > to be used directly from your code." I think this is a *big* mistake,
> > and breaks the whole idea of interfaces.
>
> Yes, but wouldn't the idea that they've chosen to implement it as an
> explicit interface hint that it shouldn't be used by user's code? So isn't
> the documentation simply confirming that?
>
> BTW, you must be using different documentation. My MSDN (Jan 2004) only
> shows the protected Dispose(bool).

Where are you looking? If you look down the index, typing BinaryWriter,
it shows BinaryWriter.IDisposable.Dispose on my box between Flush and
Null. I'm using Jan 2004 too.

> > > Besides, all it does is close the Stream which is going to happen
> > > anyway.
> >
> > It's going to happen at *some* stage, sure. I just think that getting
> > into the habit of disposing of objects which implement IDisposable and
> > thinking about the consequences of doing so is a good habit to get
> > into.
>
> Well, I think all this explicit implementation stuff simply adds a layer of
> complexity that is more annoying than helpful.

Agreed.

> I'm all for simplifying as
> much as possible. I figure if you don't want people to call a method, then
> you shouldn't implement it in a callable fashion (i.e. make it private and
> don't do it as part of the IDisposable interface to begin with).

Yup.

> I mean, you're right, if they implement IDisposable, then I should be
> expected to call Dispose(), but they're kind of saying not to do it. I mean,
> that's kind of stupid, in my mind. And in this case, it could have
> unintended consequences for the user. It screams out bad design somewhere.

Yup.

> > I would seriously consider doing so in the *very* near future. Are you
> > currently closing streams etc in finally blocks? If not, you're risking
> > problems if an exception is thrown. If you are, then your code would
> > become more readable immediately by using the using statement.
>
> I'm big on try/finally I don't know why. It adds an extra layer of
> indentation and yeah, it's kind of ugly. I just kind of got started that
> way (probably some book I read early on in my C# programming) and have been
> doing it ever since. What can I say? Habits are hard to break.

It'll come in handy if you ever do Java, where there's no using
statement (unfortunately). Still, that's definitely better than not
doing anything at all, which is what far too many people do.