- 
                Notifications
    You must be signed in to change notification settings 
- Fork 54
Add stream locations #340
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
Add stream locations #340
Conversation
d6ecb02    to
    082fc60      
    Compare
  
    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.
Thanks @gab1one! This is looking good.
There are/were three problems, the first two of which I already fixed:
- Misspelling of cutoff(only has one T).
- Compile errors with DefaultBufferedStreamHandlecallingByteBank.getMaxPos()(it was changed toByteBank.size()), and missingexists()method implementation.
- No unit tests for DefaultBufferedStreamHandle.
For (1) and (2), I fixed and force pushed the relevant commits.
For (3), could you please add a test?
The findString(..) method used to require the exact length of the handle, this length can be unknown, e.g. for compressed handles. This is no longer the case.
| Thanks for fixing this, | 
082fc60    to
    15fc988      
    Compare
  
    This basic version does not fully support random access, which will be provided by subclasses to be added in future commits.
These subinterfaces of StreamHandle provide random access over the contained streams.
| I am actually not sure if we need the  | 
9de55e6    to
    8376a59      
    Compare
  
    Some handles don't know their length and instead return -1, this commit adds an option to the length check to assure this.
8376a59    to
    183c2e4      
    Compare
  
    
These are currently used for Compressed input streams #274 and https://github.com/scijava/scijava-io-http