trace: define BatchReader with associated types
#360
Closed
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
The `TraceReader` trait uses associated types to define its `Key`, `Val`, `Time`, `Diff` but the `BatchReader` trait did not, even though they are very similar in nature. Usually the choice between asssociated types or generic parameters on a trait is determined by whether or not a particular type is expected to implement the same trait multiple times. My starting point was that these two trait should at the very least be consistent with respect to their structure and either both use generic parameters or both use associated types. All the uses in this repo (and also that I can imagine being useful) don't really need `BatchReader` to be polymorphic for a particular type and so I chose to change that one to make it consistent with `TraceReader`. The result is quite pleasing as in many cases a lot of generic parameters are erased. In order to keep this PR short I left the `Cursor` trait untouched, but I believe a similar transformation would be beneficial there too, simplifying further many type signatures. Signed-off-by: Petros Angelatos <[email protected]>
petrosagg
force-pushed
the
trace-reader-assoc-types
branch
from
1ea4170
to
3ad963f
Compare
|
superseded by #367 |
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.
The
TraceReadertrait uses associated types to define itsKey,Val,Time,Diffbut theBatchReadertrait did not, even though they are very similar in nature. Usually the choice between associated types or generic parameters on a trait is determined by whether or not a particular type is expected to implement the same trait multiple times.My starting point was that these two trait should at the very least be consistent with respect to their structure and either both use generic parameters or both use associated types.
All the uses in this repo (and also that I can imagine being useful) don't really need
BatchReaderto be polymorphic for a particular type and so I chose to change that one to make it consistent withTraceReader.The result is quite pleasing as in many cases a lot of generic parameters are erased.
In order to keep this PR short I left the
Cursortrait untouched, but I believe a similar transformation would be beneficial there too, simplifying further many type signatures.