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
API Refactoring: add execute operation to requests, extract SPI package #27
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
JulianFeinauer
approved these changes
Oct 7, 2018
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As stated on the list, I like it!
ghost
deleted the
feature/execute-operation
branch
October 26, 2018 08:09
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
API Refactoring proposal containing the following changes. The most important changes are:
Provide an
execute()
operation onPlcRequest
, returningPlcResponse
. This has already been discussed in the mailing list. The idea is to remove temporal coupling which forces the client to invoke operation in pairs: for example, creating the request off aPlcReader
and submitting the created request back to the samePlcReader
for execution.Move
PlcDriver
away from the API into the SPI package. Since execution of the requests will be implemented on the request itself, one level of indirection (PlcReader
,PlcWriter
,PlcSubscriber
,PlcProprietarySender
) in the chainPlcConnection
->PlcReader
->PlcReadRequest.Builder
->PlcReadRequest
becomes trivial and unnecessary. However, the execution of the requests will be performed differently depending on the protocol being implemented. This variation should be encapsulated behind thePlcReader
interface, but it should not be exposed to the client. Therefore, these interfaces are placed in thedriver-base
module for protocol implementers.Other minor changes include:
Remove default methods from the API. These were "workarounds" for sub-optimal API design. A public API should be as slim as possible, and default operations should be omitted.
PlcMessageBuilder
renamed toPlcRequstBuilder
. It is useful (and was acually used) only for constructing requests anyway.Reduced the amount of generics. Many types were generic, which increased the complexity dramatically, but provided almost no benefit for the client. Wherever possible, the type parameters were removed to simplify the API.