HiveBrain v1.2.0
Get Started
← Back to all entries
patterncsharpMinor

Custom TcpListener

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
customtcplistenerstackoverflow

Problem

I have only been coding C# for a short time and I can't say I'm an expert but I am rather enjoying it. I wondered if you could see any way I could improve my code.

using System;
using System.Net;
using System.Net.Sockets;

namespace Reality.Network
{
    /// 
    /// Callback to be invoked upon accepting a new connection.
    /// 
    /// Incoming socket connection
    public delegate void OnNewConnectionCallback(Socket Socket);

    /// 
    /// Reality simple asynchronous TCP listener.
    /// 
    public class SnowTcpListener : IDisposable // Snow prefix to avoid conflicts with     System.Net.TcpListener
    {
        private Socket mSocket;
        private OnNewConnectionCallback mCallback;

        public SnowTcpListener(IPEndPoint LocalEndpoint, int Backlog, OnNewConnectionCallback Callback)
        {
            mCallback = Callback;

            mSocket = new Socket(LocalEndpoint.AddressFamily, SocketType.Stream,     ProtocolType.Tcp);
            mSocket.Bind(LocalEndpoint);
            mSocket.Listen(Backlog);

            BeginAccept();
        }

        public void Dispose()
        {
            if (mSocket != null)
            { 
                mSocket.Dispose();
                mSocket = null;
            }
        }

        private void BeginAccept()
        {
            try
            {
                mSocket.BeginAccept(OnAccept, null);
            }
            catch (Exception) { }
        }

        private void OnAccept(IAsyncResult Result)
        {
            try
            {
                Socket ResultSocket = (Socket)mSocket.EndAccept(Result);
                mCallback.Invoke(ResultSocket);
            }
            catch (Exception) { }

            BeginAccept();
        }
    }
}

Solution

I'm guessing you've come from VB.

private Socket mSocket;
    private OnNewConnectionCallback mCallback;


That's not a very C# way to name private fields. C# is case sensitive, so we don't have to use those nasty prefixes. We just camelCase the names.

private Socket socket;
private OnNewConnectionCallback callback;


// Snow prefix to avoid conflicts with     System.Net.TcpListener


You don't really need to do that. You could just fully qualify the names. In this case, you might want to give the System.Net namespace an alias to make fully qualifying names a bit easier.

using Net = System.Net


Then calling the System.Net.TcpListener can be shortened up into just Net.TcpListener while your local class is called with just plain old TcpListener.

This.... don't do this.

private void BeginAccept()
{
    try
    {
        mSocket.BeginAccept(OnAccept, null);
    }
    catch (Exception) { }
}


There's two things wrong here really.

  • If you're going to catch an Exception, do something with it. Don't just swallow it and ignore it entirely.



  • You really shouldn't ever catch Exception. You should find out what exceptions could be thrown and catch only those. Otherwise, you could be catching exceptions that you can't recover from. You should only catch ones you know you can recover from.

Code Snippets

private Socket mSocket;
    private OnNewConnectionCallback mCallback;
private Socket socket;
private OnNewConnectionCallback callback;
// Snow prefix to avoid conflicts with     System.Net.TcpListener
using Net = System.Net
private void BeginAccept()
{
    try
    {
        mSocket.BeginAccept(OnAccept, null);
    }
    catch (Exception) { }
}

Context

StackExchange Code Review Q#80671, answer score: 2

Revisions (0)

No revisions yet.