Implementation of an asynchronous UDP server Listener

Question

I have written this code in C# and have tested this code to ensure it works well. Any other suggestions for this code would be appreciated as I have a number of users who are sending data on this port.

private Socket serverSocket = null;
private List<EndPoint> clientList = new List<EndPoint>();
private byte[] byteData = new byte[1024];
private int port = 1517;




private void button1_Click(object sender, EventArgs e)
{
    this.serverSocket = new Socket(AddressFamily.InterNetwork, SocketType.Dgram, ProtocolType.Udp);
    this.serverSocket.SetSocketOption(SocketOptionLevel.Socket, SocketOptionName.ReuseAddress, true);
    this.serverSocket.Bind(new IPEndPoint(IPAddress.Any, this.port));
    EndPoint newClientEP = new IPEndPoint(IPAddress.Any, 0);
    this.serverSocket.BeginReceiveFrom(this.byteData, 0, this.byteData.Length, SocketFlags.None, ref newClientEP, DoReceiveFrom, newClientEP);

}

private void DoReceiveFrom(IAsyncResult iar)
{


    try
    {
        EndPoint clientEP = new IPEndPoint(IPAddress.Any, 0);
        int dataLen = 0;
        byte[] data = null;
        try
        {
            dataLen = this.serverSocket.EndReceiveFrom(iar, ref clientEP);
            data = new byte[dataLen];
            Array.Copy(this.byteData, data, dataLen);
        }
        catch (Exception e)
        {
        }
        finally
        {
            EndPoint newClientEP = new IPEndPoint(IPAddress.Any, 0);
            this.serverSocket.BeginReceiveFrom(this.byteData, 0, this.byteData.Length, SocketFlags.None, ref newClientEP, DoReceiveFrom, newClientEP);
        }

        if (!this.clientList.Any(client => client.Equals(clientEP)))
            this.clientList.Add(clientEP);

        //     DataList.Add(Tuple.Create(clientEP, data));

        string s = Encoding.ASCII.GetString(data);


    }
    catch (ObjectDisposedException)
    {
    }
}

public void Stop()
{
    this.serverSocket.Close();
    this.serverSocket = null;
    this.clientList.Clear();
}

Answer 1

@MX D is correct in that you should not be catching exceptions, unless you are going to do something about them.

This code can and should be written without catch statements.

private void DoReceiveFrom(IAsyncResult iar)
{


    try
    {
        EndPoint clientEP = new IPEndPoint(IPAddress.Any, 0);
        int dataLen = 0;
        byte[] data = null;
        try
        {
            dataLen = this.serverSocket.EndReceiveFrom(iar, ref clientEP);
            data = new byte[dataLen];
            Array.Copy(this.byteData, data, dataLen);
        }
        catch (Exception e)
        {
        }
        finally
        {
            EndPoint newClientEP = new IPEndPoint(IPAddress.Any, 0);
            this.serverSocket.BeginReceiveFrom(this.byteData, 0, this.byteData.Length, SocketFlags.None, ref newClientEP, DoReceiveFrom, newClientEP);
        }

        if (!this.clientList.Any(client => client.Equals(clientEP)))
            this.clientList.Add(clientEP);

        //     DataList.Add(Tuple.Create(clientEP, data));

        string s = Encoding.ASCII.GetString(data);


    }
    catch (ObjectDisposedException)
    {
    }
}

you should also remove some of the empty space from the code as well.

The Commented code...what is it there for? If this is dead code then you should remove it.

Declaring a Variable at the end of this code has me a little confused....

  string s = Encoding.ASCII.GetString(data);

Get rid of that too, it isn't used anywhere in the code that you present in the Original Post

I also got rid of the Try because there isn't a need for a try if you aren't going to catch anything or finally anything. This also removes a level of unnecessary indentation.

The code now looks like this:

private void DoReceiveFrom(IAsyncResult iar)
{
    EndPoint clientEP = new IPEndPoint(IPAddress.Any, 0);
    int dataLen = 0;
    byte[] data = null;
    try
    {
        dataLen = this.serverSocket.EndReceiveFrom(iar, ref clientEP);
        data = new byte[dataLen];
        Array.Copy(this.byteData, data, dataLen);
    }
    finally
    {
        EndPoint newClientEP = new IPEndPoint(IPAddress.Any, 0);
        this.serverSocket.BeginReceiveFrom(this.byteData, 0, this.byteData.Length, SocketFlags.None, ref newClientEP, DoReceiveFrom, newClientEP);
    }

    if (!this.clientList.Any(client => client.Equals(clientEP)))
        this.clientList.Add(clientEP);
}

---

and while I am at it, I would implement the IDisposable interface and use a Dispose method instead of this

public void Stop()
{
    this.serverSocket.Close();
    this.serverSocket = null;
    this.clientList.Clear();
}

it would then look something like this

public void Dispose()
{
     this.serverSocket.Close();
     this.serverSocket = null;
     this.clientList.Clear();
}

this also gives you other options when using the class in your code.