patterncsharpMinor
Simple socket manager
Viewed 0 times
socketsimplemanager
Problem
I just wondered if someone could review this socket class for me.
Here is the
using Apple.Application.Base.Sockets;
using System;
using System.Collections.Generic;
using System.Linq;
using System.Net;
using System.Net.Sockets;
using System.Text;
using System.Threading.Tasks;
namespace Apple.Application.Base.Connection
{
class SocketManager
{
private Socket _serverSocket;
private SocketSettings _settings;
public SocketManager(SocketSettings settings)
{
_settings = settings;
_serverSocket = new Socket(AddressFamily.InterNetwork, SocketType.Stream, ProtocolType.Tcp);
BeginListen();
BeginAccept();
}
public void BeginListen()
{
_serverSocket.Bind(_settings.EndPoint);
_serverSocket.Listen(_settings.SocketBacklog);
}
public void BeginAccept()
{
_serverSocket.BeginAccept(new AsyncCallback(OnAccept), null);
}
public void OnAccept(IAsyncResult AsyncResult)
{
try
{
_settings._log.Info("New connection was received");
Apple.GetPlayerManager().RegisterNewPlayer();
}
catch (Exception exception)
{
_settings._log.Error(exception);
}
finally
{
BeginAccept();
}
}
}
}Here is the
SocketSettings class:using log4net;
using System;
using System.Collections.Generic;
using System.Linq;
using System.Net;
using System.Text;
using System.Threading.Tasks;
namespace Apple.Application.Base.Sockets
{
sealed class SocketSettings
{
public IPEndPoint EndPoint { get; set; }
public ushort SocketBacklog { get; set; }
public ILog _log { get; set; }
}
}Solution
I'd say, your call to Apple.GetPlayerManager() bothers me. Your socket implementation does not need to know this is a game and it's creating a new player etc. I whould break the relation somehow, passing the payload or event received in your socket to some other service, preferably injected using dependency injection or sth.
This would make your socket class testable on its own (by mockup objects) and other classes much easily.
PS: your SocketSettings class is actually ServerInformation, copy/paste problem I suppose..
This would make your socket class testable on its own (by mockup objects) and other classes much easily.
PS: your SocketSettings class is actually ServerInformation, copy/paste problem I suppose..
Context
StackExchange Code Review Q#109069, answer score: 2
Revisions (0)
No revisions yet.