singleFileArchive: Add DataInputStream class to decode primitive types from a byte stream.
#108
Conversation
WalkthroughThe changes introduce a new TypeScript file, Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant DataInputStream
participant DataView
Client->>DataInputStream: Create instance
DataInputStream->>DataView: Initialize with byte stream
Client->>DataInputStream: Read unsigned byte
DataInputStream->>DataView: Fetch unsigned byte
DataView-->>DataInputStream: Return byte
DataInputStream-->>Client: Return unsigned byte
Client->>DataInputStream: Read signed byte
DataInputStream->>DataView: Fetch signed byte
DataView-->>DataInputStream: Return byte
DataInputStream-->>Client: Return signed byte
Client->>DataInputStream: Seek to index
DataInputStream-->>DataInputStream: Update current position
Client->>DataInputStream: Read beyond available data
DataInputStream-->>Client: Throw DataInputStreamEOFError
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (2)
src/utils/datastream.ts (2)
25-29: Enhance clarity of error messages inDataInputStreamEOFErrorIncluding a descriptive message about the failed action improves error clarity and debugging efficiency.
Consider modifying the error message construction:
let formattedMessage = `[bufLen=${bufLen}, requiredLen=${requiredLen}]`; if ("" !== message) { formattedMessage += ` ${message}`; } + else { + formattedMessage = `DataInputStreamEOFError: Unable to read required bytes. ${formattedMessage}`; + }This ensures that if no custom message is provided, a default descriptive message is included.
50-53: DefaultisLittleEndiantotruefor consistency with common architecturesMost modern systems are little-endian. Defaulting
isLittleEndiantotruecan prevent potential confusion and align with prevalent system architectures.Update the constructor's default parameter:
- constructor (arrayBuffer: ArrayBufferLike, isLittleEndian: boolean = false) { + constructor (arrayBuffer: ArrayBufferLike, isLittleEndian: boolean = true) {
| const requiredLen: number = this.#byteIdx + length; | ||
| if (this.#dataView.byteLength < requiredLen) { | ||
| this.#byteIdx = this.#dataView.byteLength; | ||
| throw new DataInputStreamEOFError(this.#dataView.byteLength, requiredLen); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Refactor repetitive EOF checks into a helper method
Multiple methods perform similar EOF checks and error handling. Refactoring this code into a private helper method would reduce duplication and enhance maintainability.
Consider adding a helper method:
private checkEOF(length: number): void {
const requiredLen = this.#byteIdx + length;
if (this.#dataView.byteLength < requiredLen) {
this.#byteIdx = this.#dataView.byteLength;
throw new DataInputStreamEOFError(this.#dataView.byteLength, requiredLen);
}
}Then update the read methods to use this helper. For example, in readUnsignedByte():
- const requiredLen: number = this.#byteIdx + 1;
- if (this.#dataView.byteLength < requiredLen) {
- this.#byteIdx = this.#dataView.byteLength;
- throw new DataInputStreamEOFError(this.#dataView.byteLength, requiredLen);
- }
+ this.checkEOF(1);Also applies to: 106-110, 123-126, 139-142, 157-160, 175-178, 193-196, 211-214
| seek (idx: number): void { | ||
| if (idx > this.#dataView.byteLength) { | ||
| this.#byteIdx = this.#dataView.byteLength; | ||
| throw new DataInputStreamEOFError(this.#dataView.byteLength, idx); |
There was a problem hiding this comment.
Fix potential off-by-one error in the 'seek' method
The condition in the seek method should use >= instead of > to prevent seeking beyond the buffer length. Since indexing starts at zero, an index equal to the buffer length would be out of bounds and should trigger an EOF error.
Apply this diff to correct the condition:
- if (idx > this.#dataView.byteLength) {
+ if (idx >= this.#dataView.byteLength) {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| seek (idx: number): void { | |
| if (idx > this.#dataView.byteLength) { | |
| this.#byteIdx = this.#dataView.byteLength; | |
| throw new DataInputStreamEOFError(this.#dataView.byteLength, idx); | |
| seek (idx: number): void { | |
| if (idx >= this.#dataView.byteLength) { | |
| this.#byteIdx = this.#dataView.byteLength; | |
| throw new DataInputStreamEOFError(this.#dataView.byteLength, idx); |
Description
First PR in single file archive series. I am splitting up to make easier to review.
PR implements a class that can read primitive types from a serialized stream (int, long, etc…) This will be used in later PRs to decode data from serialized CLP segments.
This code existed in old log viewer (see here), and all this PR does is convert from js to typescript.
Ideally this can be merged as doesn't add any functionality, and required for other PRs.
Summary by CodeRabbit
DataInputStreamclass for enhanced data reading from byte streams, supporting multiple data types and endianness.DataInputStreamEOFError, to manage end-of-file scenarios effectively.