patterncsharpMinor
Custom TcpListener
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.
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
You don't really need to do that. You could just fully qualify the names. In this case, you might want to give the
Then calling the
This.... don't do this.
There's two things wrong here really.
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.TcpListenerYou 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.NetThen 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.TcpListenerusing Net = System.Netprivate void BeginAccept()
{
try
{
mSocket.BeginAccept(OnAccept, null);
}
catch (Exception) { }
}Context
StackExchange Code Review Q#80671, answer score: 2
Revisions (0)
No revisions yet.