patterncsharpMinor
Async TCP client/server
Viewed 0 times
asyncclienttcpserver
Problem
I'm working on a simple async TCP client server application, i'd like my network code to be reviewed. I really feel that I'm doing something wrong, especially in receiving code.
Interface for client:
Wrapper needed for client:
```
internal class PolicyWrapper
{
internal PolicyResult InitiateRetryPolicy(int retryCount, int retyDelaySeconds, Action executionTarget,
Action internalExceptionHandler)
{
var policyResult = Policy
.Handle()
//.Or(ex => ex.ParamName == "example")
.WaitAndRetry(retryCount,
i => TimeSpan.FromSeconds(retyDelaySeconds),
(exception, span) => internalExceptionHandler(exception))
.ExecuteAndCapture(executionTarget);
return policyResult;
}
public void InitiateEndlessRetryPolicy(int retyDelaySeconds, Action executionTarget,
Action internalExceptionHandler)
{
Policy
.Handle()
//.Or(ex => ex.ParamName == "example")
.WaitAndRetryForever(i => TimeSpan.FromSeconds(retyDelaySeconds),
(exception, span) => internalExceptionHandler(exception)).Execute(executionTarget);
}
}
Interface for client:
public delegate void ConnectedHandler(IAsyncClient a);
public delegate void ClientMessageReceivedHandler(IAsyncClient a, List msg);
public delegate void ClientMessageSubmittedHandler(IAsyncClient a, bool close);
public delegate void ClientErrorHandler(string errorMessage);
public interface IAsyncClient : INetworkNode, IDisposable
{
event ConnectedHandler Connected;
event ClientMessageReceivedHandler MessageReceived;
event ClientMessageSubmittedHandler MessageSubmitted;
event ClientErrorHandler Error;
void StartClient();
bool IsConnected { get; }
Task StartReceiving();
void Send(IProcessable message, bool close);
}INetworkNode:public interface INetworkNode
{
byte[] WrapMessage(byte[] message);
byte[] WrapKeepaliveMessage();
}Wrapper needed for client:
```
internal class PolicyWrapper
{
internal PolicyResult InitiateRetryPolicy(int retryCount, int retyDelaySeconds, Action executionTarget,
Action internalExceptionHandler)
{
var policyResult = Policy
.Handle()
//.Or(ex => ex.ParamName == "example")
.WaitAndRetry(retryCount,
i => TimeSpan.FromSeconds(retyDelaySeconds),
(exception, span) => internalExceptionHandler(exception))
.ExecuteAndCapture(executionTarget);
return policyResult;
}
public void InitiateEndlessRetryPolicy(int retyDelaySeconds, Action executionTarget,
Action internalExceptionHandler)
{
Policy
.Handle()
//.Or(ex => ex.ParamName == "example")
.WaitAndRetryForever(i => TimeSpan.FromSeconds(retyDelaySeconds),
(exception, span) => internalExceptionHandler(exception)).Execute(executionTarget);
}
}
Solution
Async/Await
Async/await's main advantage is that you no longer need to use
For example, this code:
Becomes
Event Notification
In the spirit of asynchronicity, I would rather expose some events which would notify interested parties that something has been sent/received than making them wait on
Single Responsibility Principle
A short look upon
The socket notifies you that some bytes have arrived over the wire. A protocol decoder receives all the bytes and splits them into meaningful messages (as mentioned in a comment, a stream of bytes doesn't mean that you receive a full message every time, it's your task to put together ["He", "llo W", "orld"] into ["Hello", "World"]). After you have a specific message, you pass it on to whatever handling mechanism you choose.
Over-engineering
I can't help noticing the large number of patterns you employ. Are you learning design-patterns? Simply throwing in all the patterns one can think off can easily lead to code that is unnecessarily complex, prepared to solve issues that might never arise at the expense of being harder to follow: YAGNI
Async/await's main advantage is that you no longer need to use
ContinueWith() to chain tasks, making the code much cleaner. Your code seems to more often favor the use of ContinueWith(). A good book or tutorial on this will help you refactor that part of the code.For example, this code:
public Task DoSomethingAsync()
{
return Task.Factory.StartNew(() =>
{
//...
return 1;
});
}
var task = DoSomethingAsync();
task.ContinueWith(value =>
{
Console.WriteLine("{0}", value);
});Becomes
public Task DoSomethingAsync()
{
return Task.Factory.StartNew(() =>
{
//...
return 1;
});
}
var value = await DoSomethingAsync();
Console.WriteLine("{0}", value);Event Notification
In the spirit of asynchronicity, I would rather expose some events which would notify interested parties that something has been sent/received than making them wait on
ManualResetEvents. Later I noticed that you also use events. In this case, why did you use the ManualResetEvent? As for the delegate type of the events, using EventHandler<> specializations makes the code more idiomatic.Single Responsibility Principle
A short look upon
StreamSecurityClient and one immediately notices that this class is doing a lot. For example, the client shouldn't be concerned in how a command is handled. The socket notifies you that some bytes have arrived over the wire. A protocol decoder receives all the bytes and splits them into meaningful messages (as mentioned in a comment, a stream of bytes doesn't mean that you receive a full message every time, it's your task to put together ["He", "llo W", "orld"] into ["Hello", "World"]). After you have a specific message, you pass it on to whatever handling mechanism you choose.
Over-engineering
I can't help noticing the large number of patterns you employ. Are you learning design-patterns? Simply throwing in all the patterns one can think off can easily lead to code that is unnecessarily complex, prepared to solve issues that might never arise at the expense of being harder to follow: YAGNI
Code Snippets
public Task<int> DoSomethingAsync()
{
return Task.Factory.StartNew(() =>
{
//...
return 1;
});
}
var task = DoSomethingAsync();
task.ContinueWith(value =>
{
Console.WriteLine("{0}", value);
});public Task<int> DoSomethingAsync()
{
return Task.Factory.StartNew(() =>
{
//...
return 1;
});
}
var value = await DoSomethingAsync();
Console.WriteLine("{0}", value);Context
StackExchange Code Review Q#138014, answer score: 6
Revisions (0)
No revisions yet.