HavenDV / H.Pipes

A simple, easy to use, strongly-typed, async wrapper around .NET named pipes.
MIT License
219 stars 26 forks source link

Generic message type <T> #18

Open alexis- opened 2 years ago

alexis- commented 2 years ago

I have noticed that all the read/write APIs can only read and write the generic type defined by the user when instantiating the client and server, eg. PipeServer<MyMessage>, PipeClient<MyMessage>.

Would you agree to a PR with the following changes:

In essence we would have something like that:

Avoiding code duplication between SingleConnectionPipeXXX and PipeXXX

Option 1: Base class

public abstract BasePipeServer : IPipeServer
{
  // Put all code shared by PipeServer and SingleConnectionPipeServer here
}
public abstract BasePipeClient : IPipeClient
{
  // Put all code shared by PipeClient and SingleConnectionPipeClient here
}

Option 2: Merge them

We delete the SingleConnectionPipeXXX classes and add an argument in the constructor:

public PipeServer(string pipeName, bool isSingleConnection = false, IFormatter? formatter = default)
public PipeClient(string pipeName, string serverName = ".", bool isSingleConnection = false, TimeSpan? reconnectionInterval = default, IFormatter? formatter = default)

Non-generic PipeXXX classes

public class PipeServer: BasePipeServer
{
  public event EventHandler<ConnectionMessageEventArgs<byte[]>>? MessageReceived;

  public async Task WriteAsync<T>(T value, CancellationToken cancellationToken = default)
  { /* Code here */ }

  public async Task WriteAsync(byte[] value, CancellationToken cancellationToken = default)
  { /* Code here */ }
}
public class PipeClient: BasePipeClient
{
  public event EventHandler<ConnectionMessageEventArgs<byte[]>>? MessageReceived;

  public async Task WriteAsync<T>(T value, CancellationToken cancellationToken = default)
  { /* Code here */ }

  public async Task WriteAsync(byte[] value, CancellationToken cancellationToken = default)
  { /* Code here */ }
}

Repeat for SingleConnectionPipeXXX.

Generic PipeXXX classes

public class PipeServer<T>: PipeServer, IPipeServer<T>
{
  public new event EventHandler<ConnectionMessageEventArgs<T?>>? MessageReceived;

  public new async Task WriteAsync(T value, CancellationToken cancellationToken = default)
  { /* Code here */ }
}
public class PipeClient<T>: PipeClient, IPipeClient<T>
{
  public new event EventHandler<ConnectionMessageEventArgs<T?>>? MessageReceived;

  public new async Task WriteAsync(T value, CancellationToken cancellationToken = default)
  { /* Code here */ }
}

Repeat for SingleConnectionPipeXXX.

Optional: Sealed classes

Optionally, it might be nice to make the PipeServer<T> and PipeClient<T> non-sealed for people who want to extend their functionalities without requiring a fork or a PR.

Optional: Sharing code between PipeServer and PipeClient

PipeServer and PipeClient also appear to share a lot of code. I could make a base class for them to avoid code duplication.

Final words

I'll be writing this for my own needs, but I would enjoy contributing to the project if possible with a PR. Let me know what you think! :)

HavenDV commented 2 years ago

Hello. Thanks for the offer, I'm not against it, it should have been done a long time ago. You will also need to make sure that tests with the current behavior of the Generic classes pass correctly and the behavior does not change. You will also need to carefully approach the issue of combining SingleConnectionPipeXXX and PipeXXX, because I do not remember the reasons for my decision to split instead of adding a parameter (but, there is a chance that they were not there and it was corny laziness) I also don't mind all Optional changes.

The only thing I'll forgive - divide the implementation into separate PRs as much as possible to simplify verification. So that there is no situation where there is no code comparison, but there are simply deleted and added files.

alexis- commented 2 years ago

Sounds good! Glad you're onboard. :)

I'll start with the generic/non-generic PipeXXX modification + tests and send a PR when it's finished.

fectus commented 1 year ago

This feature looks cool and would help me a lot to achieve polymorphic pipe messages. Apparently, the PR did not make it through though. And since it has been more than a year now the work/review has been put on hold probably.

For now, I will have to stick to sending content serialized as JSON for example and handle the JSON deserialization at the receiving side. Anyways, great and very helpful library and a very nice addition to the library API if the PR was completed.

HavenDV commented 1 year ago

I still love this suggestion. Unfortunately, the PR was quite voluminous, I was not able to quickly understand it and therefore it is still not accepted