3
\$\begingroup\$

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
{
    /// <summary>
    /// Callback to be invoked upon accepting a new connection.
    /// </summary>
    /// <param name="Socket">Incoming socket connection</param>
    public delegate void OnNewConnectionCallback(Socket Socket);

    /// <summary>
    /// Reality simple asynchronous TCP listener.
    /// </summary>
    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();
        }
    }
}
\$\endgroup\$
2
\$\begingroup\$

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.

  1. If you're going to catch an Exception, do something with it. Don't just swallow it and ignore it entirely.
  2. 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.
\$\endgroup\$
4
  • \$\begingroup\$ What exceptions should I be catching on that BeginAccept void? And thanks for your answer! \$\endgroup\$ Feb 16 '15 at 15:39
  • \$\begingroup\$ That's actually pretty hard to find out and I've never worked with the System.Net namespaces, so I've no personal experience to draw on there. I've found that the best thing to do is to not catch exceptions until you find your program is throwing one that you think you can handle in your class. Then you can use the stack trace and exception detail to know what/where to handle. \$\endgroup\$
    – RubberDuck
    Feb 16 '15 at 15:52
  • \$\begingroup\$ First port of call w.r.t. possible exceptions is MSDN docs: msdn.microsoft.com/en-us/library/5bb431f9(v=vs.110).aspx \$\endgroup\$
    – RobH
    Feb 16 '15 at 16:01
  • \$\begingroup\$ Ahhhhhh they've got them hidden down at the method level. That makes sense. Thanks @RobH. \$\endgroup\$
    – RubberDuck
    Feb 16 '15 at 16:03

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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