Using statement in context of streams and WebClients

Question

Mostly a question on which approach would be better and when utilizing the using keyword, would it still be neccesary to close streams programatically. If an object implements IDisposable should using always be used?

Link to my full answer using this sample: Stackoverflow Answer

Option 1:

WebClient webClient;
webClient = new WebClient();
string json = webClient.DownloadString(@"urlToTheJsonResponse");

MemoryStream stream = new MemoryStream((Encoding.UTF8.GetBytes(json)));
DataContractJsonSerializer ser = new DataContractJsonSerializer(typeof(GISData));
stream.Position = 0;
GISData data = (GISData)ser.ReadObject(stream);
stream.Close();

Option 2:

WebClient webClient;
MemoryStream stream;
string json;
GISData data;

using (webClient = new WebClient())
{
    json = webClient.DownloadString(@"urlToTheJsonResponse");
}

using (stream = new MemoryStream((Encoding.UTF8.GetBytes(json))))
{
    DataContractJsonSerializer ser = new DataContractJsonSerializer(typeof(GISData));
    data = (GISData)ser.ReadObject(stream);
    stream.Close();
}

using (webClient = new WebClient())
{
    webClient.DownloadFile(data.href, "C:/" + data.href.Substring(data.href.LastIndexOf("/") + 1));
}

Answer 1

Under normal circumstances, you should always either call Dispose() explicitly or use a using block. This applies especially for streams and related types (like StreamWriter), where not disposing can have very visible bad consequences (e.g. the end of the text won't be written to the file).

But there are some types for which calling Dispose() doesn't actually do anything useful. And WebClient and MemoryStream are among them. They are IDisposable primarily because they inherit that interface from their base class (Component and Stream, respectively). So, I think your Option 1 is fine.

If you are unsure whether you should dispose objects of some type or whether it's one of the few types where not disposing is safe, err on the side of caution and dispose it.

Answer 2

I realize this is over two years later, but I thought I should point out a redundancy in the code in the OP. I see this:

using (stream = new MemoryStream((Encoding.UTF8.GetBytes(json)))) {
  DataContractJsonSerializer ser = new DataContractJsonSerializer(typeof(GISData));
  data = (GISData)ser.ReadObject(stream);
  stream.Close();
}

Notice that stream.Close(); at the end of the block - which, of course, is unnecessary since stream is the object of the using.

(I would have just used comment under the OP, but I just signed up to this particular branch of stackexchange and don't have the rep requirement.)

Answer 3

Yes you should, classes which implement IDisposable are basically stating that they internally have used managed or unmanaged resources which require explicit tidy up outside of the remit of the garbage collector. The using statement is simply a compiler short cut to a try/finally block which ensures that the dispose method is called even if the code inside the using block throws an exception.

using (webClient = new WebClient())
{
    webClient.DownloadFile(data.href, "C:/" + data.href.Substring(data.href.LastIndexOf("/") + 1));
}

becomes

WebClient webClient = new WebClient();
try
{
    webClient.DownloadFile(data.href, "C:/" + data.href.Substring(data.href.LastIndexOf("/") + 1));
}
finally
{
    if (webClient != null)
    {
        webClient.Dispose();
    }
}

Although the garbage collector will call the Finalize() method on any IDisposable instance that implements one, that won't happen until the object is available for garbage collection and therefore the tidy up won't happen until some point in the future.

Most classes with a finalizer will call the dispose method but usually only tidy up unmanaged resources when called from the finalizer, therefore it is still recommended to call dispose yourself as soon as you are finished with an object to ensure that you don't hold on to other resources longer than necessary.

Answer 4

Yes, always. Unless it's a class-level variable in which your type should then implement IDisposable and dispose of them in its Dispose() method.

So, option 2.