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

Manage creation and disposal of ITcpConnection in AprsIsConnection #124

Merged
merged 8 commits into from
Apr 26, 2022

Conversation

CBielstein
Copy link
Owner

Description

Currently, as unit tests require the ability to inject an ITcpConnection, AprsIsConnection has a single constructor which requires specifying an ITcpConnection.

However, most clients will not need or want to specify their own ITcpConnection, so this is overly cumbersome. Instead, this PR changes the incoming ITcpConnection to be optional.

To handle ambiguity around if the ITcpConnection should be disposed, we follow the pattern laid out by Microsoft's own HttpClient, which allows injection of an HttpMessageHandler and specifying whether the HttpClient or the caller should dispose the handler.

For most clients of AprsIsConnection, they should simply call new AprsIsConnection() and allow the object to create and dispose resources. For special cases (tests or advanced resource management), they can call the injection constructor with their ITcpConnection and instructions on if it should be disposed.

Changes

  • Add an AprsIsConnection constructor overload to allow not passing in an ITcpConnection
  • Update the AprsIsConnection(ITcpConnection) constructor to AprsIsConnection(ITcpConnection, bool) to allow specifying who should dispose the ITcpConnection
  • Make AprsIsConnection disposable to facilitate destruction of resources and handle disposes in code and tests
  • Move IDisposable implementation from TcpConnection to the interface ITcpConnection to facilitate dispose code
  • Added a test to ensure ITcpConnection.Dispose() is only called at the right times

Validation

  • Added new test around AprsIsConnection.Dispose() logic
  • All tests and build passing

@CBielstein CBielstein added enhancement New feature or request code cleanliness Refactors, static analysis, etc. labels Apr 18, 2022
@CBielstein CBielstein self-assigned this Apr 18, 2022
@CBielstein CBielstein merged commit 5597e73 into main Apr 26, 2022
@CBielstein CBielstein deleted the managed-tcpclient branch April 26, 2022 03:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
code cleanliness Refactors, static analysis, etc. enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant