1
\$\begingroup\$

I basically have a couple of clients built with C#, using System.Net.Sockets TCP connections connecting to a node server backend.

I've looked at tons of examples and everyone seems to receive data in a different way. I copied methods I liked and thought would work for me and put together this method.

This is the method I use for receiving ALL data from the server. Is this method acceptable? Is there anything I can do better here?

ReceiveData Method

/// <summary>
/// This should be started in it's own thread as it blocks like crazy
/// This will keep running while the socket is connected, waiting for
/// data to arrive and then handle it (via calling the action you pass)
/// </summary>
/// <param name="tcpNetwork">the tcp connection which contains the open socket and stream reader</param>
/// <param name="handleFunction">Action in which you want to run to handle your packets received</param>
public static void ReceiveData(TcpNetwork tcpNetwork, Action<Packet> handleFunction)
{
    //grab the streamReader from the tcp object
    var streamReader = tcpNetwork.GetStreamReader();

    //while the socket is still connected
    while (tcpNetwork.Connected)
    {
        try
        {
            //Create a new header object which is a static size 
            //(Set Via TcpNetwork)
            //the server will always send a header packet before any
            //other packet
            var header = new char[TcpNetwork.HeaderLength];

            //attempt to read the header, this will find data, or
            //throw an IOException when we timeout (if no data is being sent)
            //in which case we just hide it and start looking for more data
            TcpNetwork.ReadWholeArray(streamReader, header);

            //if we got this far that means we have a header packet (in json form)
            //convert it to our HeaderPacket object so we can determine the length of the
            //content packet
            var headerPacket = JsonConvert.DeserializeObject<HeaderPacket>(new string(header));

            //create a new char array for our content packet using the size that was sent 
            //with our header packet
            var contentPacket = new char[headerPacket.Length];

            //attempt to read the whole contentPacket, we will keep reading until we get the right amount of data
            //or we reach the end of the stream in which case we'll throw an exception cause something bad happened
            TcpNetwork.ReadWholeArray(streamReader, contentPacket);

            //convert our character array to a string
            var json = new string(contentPacket);

            //make sure the string is json
            if (Packet.IsJson(json))
            {
                //convert it from json to our packet object
                var packet = Packet.ConvertFromServerPacket(json);

                //call the action we passed in to handle all the packets
                handleFunction(packet);
            }
            else
            {
                throw new FormatException("Received non-json response from server");
            }
        }
        catch (IOException ioException)
        {
            //natural timeout if we don't receive any messages
            //keep chugging
        }
        catch (ThreadAbortException abortingExcetion)
        {
            // -- let it abort
        }
        catch (Exception x)
        {
            throw x;
        }
    } 
}

Relevant parts of TcpNetwork.cs

/// <summary>
/// Reads data into a complete array, throwing an EndOfStreamException
/// if the stream runs out of data first, or if an IOException
/// naturally occurs.
/// </summary>
/// <param name="reader">The stream to read data from</param>
/// <param name="data">The array to read bytes into. The array
/// will be completely filled from the stream, so an appropriate
/// size must be given.</param>
public static void ReadWholeArray(StreamReader reader, char[] data)
{
    var offset = 0;
    var remaining = data.Length;
    while (remaining > 0)
    {
        var read = reader.Read(data, offset, remaining);
        if (read <= 0)
            throw new EndOfStreamException
                (String.Format("End of stream reached with {0} bytes left to read", remaining));
        remaining -= read;
        offset += read;
    }
}

/// <summary>
/// Gets a StreamReader which can be used to read from the connected socket
/// </summary>
/// <returns>StreamReader of a network stream</returns>
public StreamReader GetStreamReader()
{
    if (_tcpClient == null)
        return null;

    return _streamReader ?? (_streamReader = new StreamReader(_tcpClient.GetStream()));
}

HeaderPacket.cs

public class HeaderPacket
{
    public int Length { get; set; }
}

Clarifications

  • TcpNetwork is a wrapper class around System.Net.Sockets.TcpClient
  • Server is node.js so sending JSON across the wire seemed reasonable. I'm not sure if that's something I should be doing or not.
  • Using Json.net to convert between my POCOs and JSON objects
\$\endgroup\$

1 Answer 1

2
\$\begingroup\$

Just one minor issue: You shouldn't rethrow an exception with throw ex;, because this messes up the stack trace. It is recommended to use just throw; instead.

Out of personal preference I wouldn't write that much of comments, actually. Sure, it looks nice, but you are explaining twice what ReadWholeArray does. I think the name is chosen well enough to give an idea what the method does, and for details you have the documentation at the definition of the method (e.g. shown via IntelliSense). Comments shouldn't explain how exactly you are doing something, they should tell the reader what you are doing, and why.

\$\endgroup\$
1
  • \$\begingroup\$ thanks for pointing that out about the throw. As far as the comments, yeah, i never really comment things that much, but when building it i kept forgetting what things are and how they worked, so i kinda just added them so I could walk through what was happening, but good point \$\endgroup\$
    – Kyle
    Jun 29, 2013 at 14:00

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.