Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Non-generic pipes #19

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

alexis-
Copy link

@alexis- alexis- commented Apr 23, 2022

Hello!

As discussed:

  • Added: Non-generic versions of PipeServer, PipeClient, SingleConnectionPipeServer, SingleConnectionPipeClient
  • Added: Tests for the new PipeXXX classes in the form of [DataRow()], see DataTests.cs
  • Added: FormatterExtensions.cs which implements extensions methods SerializeAsync and DeserializeAsync on objects
  • Added: New IPipeConnection interface for PipeConnection
  • Updated: Generic versions of PipeXXX inherit from their non-generic counterpart
  • Updated: Renamed IPipeConnection to IPipe since all PipeXXX inherit from them (the name seemed more fitting)
  • Updated: BaseTests.cs to test both the Generic and Non-generic PipeXXX versions
  • Updated: Made PipeXXX non-sealed
  • Updated: PipeXXX private methods and properties are now protected and documented

…tionPipeServer, SingleConnectionPipeClient

- Added: Tests for the new PipeXXX classes in the form of [DataRow()], see DataTests.cs
- Added: FormatterExtensions.cs which  implements extensions methods SerializeAsync and DeserializeAsync on objects
- Added: New IPipeConnection interface for PipeConnection
- Updated: Generic versions of PipeXXX inherit from their non-generic counterpart
- Updated: Renamed IPipeConnection to IPipe since all PipeXXX inherit from them (the name seemed more fitting)
- Updated: BaseTests.cs to test both the Generic and Non-generic PipeXXX versions
- Updated: Made PipeXXX non-sealed
- Updated: PipeXXX private methods and properties are now protected and documented
@alexis-
Copy link
Author

alexis- commented Apr 23, 2022

#18

@alexis-
Copy link
Author

alexis- commented Apr 24, 2022

P.S. static lambdas won't work here because they're using variables from the outer scope

@HavenDV
Copy link
Owner

HavenDV commented Apr 24, 2022

In any case, thank you for the changes. Give me a few days to delve into them in detail and think through possible problems.

@HavenDV
Copy link
Owner

HavenDV commented Apr 29, 2022

In fact, an amazing amount of work has been done. Thank you for that.
In some moments of implementation, I see omissions. To speed up the process, I'll just change it, and in the future we can discuss the changes, if suddenly I made a mistake in something.

@HavenDV
Copy link
Owner

HavenDV commented Apr 29, 2022

As for function parameters - they are aligned well, but I'm not sure if that's practical, as long as there is no automatic option to align that. Perhaps you know of something that automates this?

@alexis-
Copy link
Author

alexis- commented Apr 29, 2022

In some moments of implementation, I see omissions. To speed up the process, I'll just change it, and in the future we can discuss the changes, if suddenly I made a mistake in something.

Sounds good. :)

As for function parameters - they are aligned well, but I'm not sure if that's practical, as long as there is no automatic option to align that. Perhaps you know of something that automates this?

Ah yes, that's a good point. I'm using Resharper which does almost all the formatting for me. It has become an automatism, and even though I tried to adhere to the coding style in the project, I didn't realize parameter alignment would be awkward. I trust your judgement, so please do what you think is best!

@HavenDV
Copy link
Owner

HavenDV commented Apr 29, 2022

Ah yes, that's a good point. I'm using Resharper which does almost all the formatting for me. It has become an automatism, and even though I tried to adhere to the coding style in the project, I didn't realize parameter alignment would be awkward. I trust your judgement, so please do what you think is best!

Oh, after I started working on large projects, using ReSharper became painful (it really slows down the IDE on large projects). Gotta give it another chance :)

@alexis-
Copy link
Author

alexis- commented May 10, 2022

Hello Konstantin! :)

I have made a minor addition that I would like to submit. It simply consists of overloads to the WriteAsync methods which accept offset and length parameters.

This is very useful for optimizing memory allocation and buffer usages, and will align H.Pipes with the API surface of most networking libraries.

I have added some test code and also made some minor improvement to the documentation.

Should I push those changes on this PR?

Cheers,
Alexis

@alexis-
Copy link
Author

alexis- commented May 10, 2022

I have pushed the changes in a separate branch if you wish to check it out! It is based on this very branch.

See commit: alexis-@0f3dbbd

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants