What to include in using statement?

[WDrago]

All,

Is this adeqaute to close all resources associated with clientSocket?

 public void readAll(String[] s)
        {
            using (TcpClient clientSocket = new TcpClient(host, port))
            {
                NetworkStream clientStream = clientSocket.GetStream();
                StreamWriter toOmega = new StreamWriter(clientStream);
                StreamReader fromOmega = new StreamReader(clientStream);

                toOmega.Write(readTemp);
                toOmega.Flush(); //Command is not sent until buffer is flushed
                s[0] = fromOmega.ReadLine();

                toOmega.Write(readHum);
                toOmega.Flush();
                s[1] = fromOmega.ReadLine();              
            }
        }

Or should I do this (seems overkill):

 public void readAll(String[] s)
        {
            using (TcpClient clientSocket = new TcpClient(host, port))
            using (NetworkStream clientStream = clientSocket.GetStream())
            using (StreamWriter toOmega = new StreamWriter(clientStream))
            using (StreamReader fromOmega = new StreamReader(clientStream))
            {
                toOmega.Write(readTemp);
                toOmega.Flush(); //Command is not sent until buffer is flushed
                s[0] = fromOmega.ReadLine();

                toOmega.Write(readHum);
                toOmega.Flush();
                s[1] = fromOmega.ReadLine();
            }
        }

Thanks,
-Bill

[deceptikon]

Or should I do this (seems overkill):

You should do that. It may seem like overkill to you now, but a using statement (implicitly calls Dispose()) will flush the streams, close them, and dispose of any internally allocated resources. To do it manually would require calling Dispose() manually, which could cause problems in the presence of exceptions.

[WDrago]

Okay, deceptikon...that makes sense. I wasn't sure if disposing of clientSocket would automatically dispose of the NetworkStream and the StreamReader/Writers since they are all based on the client socket. I figured once clientSocket was gone then everything else would close automatically.

Thanks,
-Bill

[Ketsuekiame]

As a preference to formatting I would also nest it all, but that's just me ;)

deceptikon: A dispose method should be coded so that it cannot throw an exception, or, if it does, bin out of the application. The only exception to this rule is ObjectDisposedException, which means you have a code/design issue.
If the finalizer hits dispose and it throws an exception, it will terminate your process anyway.

A secondary problem of throwing exceptions in dispose, especially in using statements, is that they override any original exceptions.

public class DisposableObject : IDisposable
{
    /* class stuff and most of IDisposable implementation */

    Dispose()
    {
        throw new Exception("What could possibly go wrong?");
    }
}

{
    /* code... */
    try
    {
        using(IDisposable someObject = new DispoableObject())
        {
            throw new NotImplementedException("YOU MUST WRITE CODE!");
        }
    }catch(Exception exc)
    {
        Console.WriteLine(exc.Message);
    }
}




Output: What could possibly go wrong?

Your dispose exception has now hidden the original exception. Now image that the exception is thrown a little down the stack, it becomes very hard to trace what the problem is.
This is especially true in WCF programming. This is so badly written that exceptions are hidden one after the other and at the end of the chain all you get is a non-sensical error message that sends you off in the wrong debugging direction.

Having been bitten (and hard) by it, unless I trust what I put in the using statement, I will dispose manually.

In answer to the ops second post; the stream is disposed of but the parent objects are still accessible, however, they will throw exceptions saying the stream is closed. It's better to make sure Dispose gets call on everything.

[deceptikon]

deceptikon: A dispose method should be coded so that it cannot throw an exception, or, if it does, bin out of the application

My comment wasn't about exceptions thrown from within Dispose(). My concern was more general in that all of your calls to Dispose() may not happen if intervening code throws. For example:

bar.Dispose();
Console.WriteLine("bar disposed");
foo.Dispose();
Console.WriteLine("foo disposed");

If the first WriteLine() throws, foo.Dispose() won't execute. A using statement fixes this

using (var foo = MakeFoo())
{
    using (var bar = MakeBar())
    {
        // Stuff that can throw
    }

    Console.WriteLine("bar disposed");

    // Stuff that can throw
}

Console.WriteLine("foo disposed");

by forcing all of the necessary Dispose() calls to execute in a finally statement:

{
    var foo = MakeFoo();

    try
    {
        var bar = MakeBar;

        try
        {
            // Stuff that can throw
        }
        finally
        {
            if (bar != null)
                bar.Dispose();
        }

        Console.WriteLine("bar disposed");
    }
    finally
    {
        if (foo != null)
            foo.Dispose();
    }

    Console.WriteLine("foo disposed");
}

[Ketsuekiame]

Ahh, fair enough. I read it in a different way than you intended, but I'd still rather do it manually, for the reasons I gave above.

using does make it more readable, but you sacrifice some control. Depends on your needs and requirements :)

[WDrago]

As a preference to formatting I would also nest it all, but that's just me ;)

Does it make a functional difference or is it just a formatting preference?

Like this:

 public void readAll(String[] s)
        {
            using (TcpClient clientSocket = new TcpClient(host, port))
            {
                using (NetworkStream clientStream = clientSocket.GetStream())
                {
                    using (StreamWriter toOmega = new StreamWriter(clientStream))
                    {
                        // Temperature
                        toOmega.Write(readTemp);
                        toOmega.Flush(); //Command is not sent until buffer is flushed
                        using (StreamReader fromOmega = new StreamReader(clientStream))
                        {
                            s[0] = fromOmega.ReadLine();
                        }

                        // Humidity
                        toOmega.Write(readHum);
                        toOmega.Flush();
                        using (StreamReader fromOmega = new StreamReader(clientStream))
                        {
                            s[1] = fromOmega.ReadLine();
                        }
                    }
                }
    }

Or like this (I don't even know if this will work--do the repsonses pile up in a buffer that can be read sequentially like this?):

using (TcpClient clientSocket = new TcpClient(host, port))
            {
                using (NetworkStream clientStream = clientSocket.GetStream())
                {
                    using (StreamWriter toOmega = new StreamWriter(clientStream))
                    {
                        // Temperature
                        toOmega.Write(readTemp);
                        toOmega.Flush(); //Command is not sent until buffer is flushed

                        // Humidity
                        toOmega.Write(readHum);
                        toOmega.Flush();
                        using (StreamReader fromOmega = new StreamReader(clientStream))
                        {
                            s[0] = fromOmega.ReadLine();
                            s[1] = fromOmega.ReadLine();
                        }                        
                    }
                }

Thanks a bunch for your thoughtful respsonses. I really appreciate being able to learm from more knowledgeable people...

-Bill

[deceptikon]

Does it make a functional difference or is it just a formatting preference?

It's a formatting preference, assuming I understand your question. C# inherits a C rule wherein if you don't include braces on a compound statement (eg. if, while, or using), the next statement constitutes the body. So you can chain usings like you did:

using (TcpClient clientSocket = new TcpClient(host, port))
    using (NetworkStream clientStream = clientSocket.GetStream())
        using (StreamWriter toOmega = new StreamWriter(clientStream))
            using (StreamReader fromOmega = new StreamReader(clientStream))
            {
                ...
            }

And the functionality is as if you did this:

using (TcpClient clientSocket = new TcpClient(host, port))
{
    using (NetworkStream clientStream = clientSocket.GetStream())
    {
        using (StreamWriter toOmega = new StreamWriter(clientStream))
        {
            using (StreamReader fromOmega = new StreamReader(clientStream))
            {
                ...
            }
        }
    }
}

Because the braces aren't needed for all but the innermost statement (unless its body only consists of a single statement), you'll often see an unindented chain because it tends to look cleaner than the unbraced indented version:

using (TcpClient clientSocket = new TcpClient(host, port))
using (NetworkStream clientStream = clientSocket.GetStream())
using (StreamWriter toOmega = new StreamWriter(clientStream))
using (StreamReader fromOmega = new StreamReader(clientStream))
{
    ...
}

But some people (myself included) feel that this obscures the structure of the code, and prefer the fully braced and indented version.

do the repsonses pile up in a buffer that can be read sequentially like this?

Yes. You might consider using WriteLine() instead of Write() though, so that ReadLine() finds a newline to stop on. Otherwise it'll just keep reading until the stream is empty, which would result in you reading both buffered lines into s[0] and null into s[1].

[WDrago]

But some people (myself included) feel that this obscures the structure of the code, and prefer the fully braced and indented version.

I'm with you. I like it better than the unbraced verson.

Thanks again to both of you for the help.

Regards,
-Bill