Add InputStreamBase: common base class of InputBinaryStream and InputTextStream #1426
Conversation
This file contains hidden or 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
N-Dekker
force-pushed
the
InputStreamBase
branch
2 times, most recently
from
e3813ec to
6277983
Compare
There was a problem hiding this comment.
Pull Request Overview
This PR introduces a new base class InputStreamBase to provide common functionality for InputBinaryStream and InputTextStream classes, reducing code duplication and improving maintainability. The refactoring consolidates shared stream management logic into the base class while maintaining the same public API.
Key changes:
- Created
InputStreamBaseas a common base class with shared stream management functionality - Refactored
InputTextStreamandInputBinaryStreamto inherit from the base class - Consolidated the
lexical_castfunction implementation into the base class
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
include/itkInputStreamBase.h |
New base class containing common stream management methods and member variables |
include/itkInputTextStream.h |
Refactored to inherit from InputStreamBase, removing duplicated code |
include/itkInputBinaryStream.h |
Refactored to inherit from InputStreamBase, removing duplicated code |
src/itkInputStreamBase.cxx |
Updated to implement lexical_cast for the base class instead of text stream |
src/itkInputBinaryStream.cxx |
Removed file as functionality moved to base class |
src/CMakeLists.txt |
Updated build configuration to include base class and remove binary stream source |
N-Dekker
force-pushed
the
InputStreamBase
branch
5 times, most recently
from
4f2ba74 to
3a08ecd
Compare
Paved the way to share its source code with InputBinaryStream.
Very much reduces the amount of duplicate code. Adds the ability to call `GetPointer()` on an InputBinaryStream, just like it was already supported on an InputTextStream.
N-Dekker
force-pushed
the
InputStreamBase
branch
from
3a08ecd to
6aac66e
Compare
N-Dekker
added a commit
to N-Dekker/ITK-Wasm
that referenced
this pull request
Jul 24, 2025
Follow-up to pull request InsightSoftwareConsortium#1426 commit b737cf4 "feat: Add InputStreamBase, base class of InputTextStream"
N-Dekker
added a commit
to N-Dekker/ITK-Wasm
that referenced
this pull request
Jul 24, 2025
Follow-up to pull request InsightSoftwareConsortium#1426 commit 6aac66e "feat: Let InputBinaryStream inherit from InputStreamBase"
N-Dekker
added a commit
to N-Dekker/ITK-Wasm
that referenced
this pull request
Jul 25, 2025
Follow-up to pull request InsightSoftwareConsortium#1426 commit 6aac66e "feat: Let InputBinaryStream inherit from InputStreamBase"
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.
Aims to share the common source code between InputBinaryStream and InputTextStream.