feat(io_interface): Port Reader and Writer interfaces from CLP with minimal compilation fixes.

[Bill-hbrhbr]

Description

This PR serves as a preliminary step toward standardizing io_interface as part of the ystdlib-cpp library. This will help us provide a clear view of the changes we are going to make compared to CLP's main branch. Moving forward, we plan to update the API to use the new error_handling utilities, such as outcome and Result.

The list of changed items are:

• Namespace changes from clp to ystdlib::io_interface.
• Header file header guard changes to reflect the new file paths.
• Simplify OperationFailed by making it inherit from std::exception. We have pending changes in feat(error_handling): Migrate TraceableException from CLP with changes: #43
  • Simplify all throw sites too by removing __FILENAME__ and __LINE__
• Remove non-existent header includes like TraceableException.hpp and Defs.hpp
• Put NOLINTBEGIN and NOLINTEND around cpp files as we defer clang-tidy processing to the refactor stage.
• Since the CLP main branch doesn't include any test files that directly test these interface classes, we've added placeholder test_ReaderInterface.cpp and test_WriterInterface.cpp files for now. This is necessary because TESTS_SOURCES is required when building ystdlib libraries.

Checklist

• The PR satisfies the contribution guidelines.
• This is a breaking change and that has been indicated in the PR title, OR this isn't a
  breaking change.
• Necessary docs have been updated, OR no docs need to be updated.

Validation performed

• No tests added.

Appendix

Here's the diff for each ported file.

ReaderInterface.hpp

1,2c1,3
< #ifndef CLP_READERINTERFACE_HPP
< #define CLP_READERINTERFACE_HPP
---
> #ifndef YSTDLIB_IO_INTERFACE_READERINTERFACE_HPP
> #define YSTDLIB_IO_INTERFACE_READERINTERFACE_HPP
> // NOLINTBEGIN
7d7
< #include "Defs.h"
9d8
< #include "TraceableException.hpp"
11c10
< namespace clp {
---
> namespace ystdlib::io_interface {
15c14
<     class OperationFailed : public TraceableException {
---
>     class OperationFailed : public std::exception {
17,22c16
<         // Constructors
<         OperationFailed(ErrorCode error_code, char const* const filename, int line_number)
<                 : TraceableException(error_code, filename, line_number) {}
<
<         // Methods
<         char const* what() const noexcept override { return "ReaderInterface operation failed"; }
---
>         OperationFailed(ErrorCode error_code) {}
148c142
<         throw OperationFailed(error_code, __FILENAME__, __LINE__);
---
>         throw OperationFailed(error_code);
152c146
< }  // namespace clp
---
> }  // namespace ystdlib::io_interface
154c148,149
< #endif  // CLP_READERINTERFACE_HPP
---
> // NOLINTEND
> #endif  // YSTDLIB_IO_INTERFACE_READERINTERFACE_HPP

ReaderInterface.cpp

0a1
> // NOLINTBEGIN
5c6
< namespace clp {
---
> namespace ystdlib::io_interface {
51c52
<         throw OperationFailed(error_code, __FILENAME__, __LINE__);
---
>         throw OperationFailed(error_code);
62c63
<         throw OperationFailed(error_code, __FILENAME__, __LINE__);
---
>         throw OperationFailed(error_code);
87c88
<         throw OperationFailed(error_code, __FILENAME__, __LINE__);
---
>         throw OperationFailed(error_code);
105c106
<         throw OperationFailed(error_code, __FILENAME__, __LINE__);
---
>         throw OperationFailed(error_code);
113c114
<         throw OperationFailed(error_code, __FILENAME__, __LINE__);
---
>         throw OperationFailed(error_code);
121c122
<         throw OperationFailed(error_code, __FILENAME__, __LINE__);
---
>         throw OperationFailed(error_code);
126c127,128
< }  // namespace clp
---
> }  // namespace ystdlib::io_interface
> // NOLINTEND

WriterInterface.hpp

1,2c1,3
< #ifndef CLP_WRITERINTERFACE_HPP
< #define CLP_WRITERINTERFACE_HPP
---
> #ifndef YSTDLIB_IO_INTERFACE_WRITERINTERFACE_HPP
> #define YSTDLIB_IO_INTERFACE_WRITERINTERFACE_HPP
> // NOLINTBEGIN
8d8
< #include "TraceableException.hpp"
10c10
< namespace clp {
---
> namespace ystdlib::io_interface {
14c14
<     class OperationFailed : public TraceableException {
---
>     class OperationFailed : public std::exception {
16,21c16
<         // Constructors
<         OperationFailed(ErrorCode error_code, char const* const filename, int line_number)
<                 : TraceableException(error_code, filename, line_number) {}
<
<         // Methods
<         char const* what() const noexcept override { return "WriterInterface operation failed"; }
---
>         OperationFailed(ErrorCode error_code) {}
80c75
< }  // namespace clp
---
> }  // namespace ystdlib::io_interface
82c77,78
< #endif  // CLP_WRITERINTERFACE_HPP
---
> // NOLINTEND
> #endif  // YSTDLIB_IO_INTERFACE_WRITERINTERFACE_HPP

WriterInterface.cpp

0a1
> // NOLINTBEGIN
3,5c4
< #include "Defs.h"
<
< namespace clp {
---
> namespace ystdlib::io_interface {
17c16
<         throw OperationFailed(error_code, __FILENAME__, __LINE__);
---
>         throw OperationFailed(error_code);
24c23
<         throw OperationFailed(error_code, __FILENAME__, __LINE__);
---
>         throw OperationFailed(error_code);
32c31
<         throw OperationFailed(error_code, __FILENAME__, __LINE__);
---
>         throw OperationFailed(error_code);
37c36,37
< }  // namespace clp
---
> }  // namespace ystdlib::io_interface
> // NOLINTEND

ErrorCode.hpp

1,2c1,2
< #ifndef CLP_ERRORCODE_HPP
< #define CLP_ERRORCODE_HPP
---
> #ifndef YSTDLIB_IO_INTERFACE_ERRORCODE_HPP
> #define YSTDLIB_IO_INTERFACE_ERRORCODE_HPP
4c4,6
< namespace clp {
---
> // NOLINTBEGIN
>
> namespace ystdlib::io_interface {
28c30
< }  // namespace clp
---
> }  // namespace ystdlib::io_interface
30c32,33
< #endif  // CLP_ERRORCODE_HPP
---
> // NOLINTEND
> #endif  // YSTDLIB_IO_INTERFACE_ERRORCODE_HPP

Summary by CodeRabbit

• New Features
  • Introduced a new module that provides enhanced data input and output capabilities.
  • Added robust interfaces for managing data reading and writing with comprehensive error handling.
  • Expanded the project’s build configuration to integrate this new, modular functionality.

[coderabbitai]

Walkthrough

This pull request adds a new I/O interface module to the project. The changes update the CMake configuration to include the new io_interface subdirectory and introduce a set of new files within that module. These files define a library with an error enumeration, reader, and writer interfaces along with their implementations and tests. The reader and writer components include multiple methods for handling I/O operations with proper error reporting and exception handling.

Changes

File(s)	Change Summary
src/ystdlib/CMakeLists.txt	Added subdirectory inclusion for io_interface.
src/ystdlib/io_interface/CMakeLists.txt	Introduced a new CMake configuration for the io_interface library, specifying public headers, private sources, and test sources.
src/ystdlib/io_interface/ErrorCode.hpp	Added header defining the ErrorCode enumeration in the ystdlib::io_interface namespace with 21 distinct error codes.
src/ystdlib/io_interface/ReaderInterface.{hpp,cpp}	Added new ReaderInterface class and implementation with multiple methods for reading operations, including robust error checking and exception handling.
src/ystdlib/io_interface/WriterInterface.{hpp,cpp}	Added new WriterInterface class and implementation with methods for writing characters/strings and managing stream positions, including proper error checking with exceptions.

Sequence Diagram(s)

sequenceDiagram
    participant Client as Client
    participant Reader as ReaderInterface
    participant IO as I/O Subsystem

    Client->>Reader: call read()/try_read_to_delimiter()...
    activate Reader
    Reader->>IO: issue read operation
    IO-->>Reader: return data or error code
    alt Successful Read
        Reader-->>Client: return data
    else Read Error
        Reader-->>Client: throw OperationFailed exception / return error
    end
    deactivate Reader

Loading

sequenceDiagram
    participant Client as Client
    participant Writer as WriterInterface
    participant IO as I/O Subsystem

    Client->>Writer: call write_char()/write_string()
    activate Writer
    Writer->>IO: perform write operation
    IO-->>Writer: acknowledge write or return error
    alt Successful Write
        Writer-->>Client: confirm success
    else Write Error
        Writer-->>Client: throw OperationFailed exception
    end
    deactivate Writer

Loading

Suggested reviewers

• kirkrodrigues
• LinZhihao-723
• davidlion

✨ Finishing Touches

• 📝 Generate Docstrings

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

• Review comments: Directly reply to a review comment made by CodeRabbit. Example:
  • I pushed a fix in commit <commit_id>, please review it.
  • Generate unit testing code for this file.
  • Open a follow-up GitHub issue for this discussion.
• Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
  • @coderabbitai generate unit testing code for this file.
  • @coderabbitai modularize this function.
• PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
  • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
  • @coderabbitai read src/utils.ts and generate unit testing code.
  • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
  • @coderabbitai help me debug CodeRabbit configuration file.

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)

• @coderabbitai pause to pause the reviews on a PR.
• @coderabbitai resume to resume the paused reviews.
• @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
• @coderabbitai full review to do a full review from scratch and review all the files again.
• @coderabbitai summary to regenerate the summary of the PR.
• @coderabbitai generate docstrings to generate docstrings for this PR.
• @coderabbitai resolve resolve all the CodeRabbit review comments.
• @coderabbitai plan to trigger planning for file edits and PR creation.
• @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
• @coderabbitai help to get help.

Other keywords and placeholders

• Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
• Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
• Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

• You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
• Please see the configuration documentation for more information.
• If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

• Visit our Documentation for detailed information on how to use CodeRabbit.
• Join our Discord Community to get help, request features, and share feedback.
• Follow us on X/Twitter for updates and announcements.

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (8)

src/ystdlib/io_interface/ErrorCode.hpp (2)

7-29: Consider using enum class for modern C++ type safety

While the C-style enum declaration works, using enum class ErrorCode would provide better type safety by preventing implicit conversions and reducing namespace pollution, which aligns with modern C++ practices.

- typedef enum {
+ enum class ErrorCode {
-     ErrorCode_Success = 0,
+     Success = 0,
-     ErrorCode_BadParam,
+     BadParam,
-     ErrorCode_BadParam_DB_URI,
+     BadParam_DB_URI,

This would require updating all references to use the scoped enum values (e.g., ErrorCode::Success instead of ErrorCode_Success).

---

7-29: Add documentation for error codes

Consider adding documentation comments for each error code to describe their purpose and usage scenarios, especially for specific errors like ErrorCode_BadParam_DB_URI and ErrorCode_Failure_DB_Bulk_Write.

src/ystdlib/io_interface/WriterInterface.cpp (2)

9-11: Consider documenting null character handling in write_string

The current implementation uses c_str() which means that if the string contains null characters, only characters up to the first null character might be written, depending on the underlying implementation of write. Consider documenting this behavior or ensuring it's consistent with expectations.

---

27-35: LGTM, with a minor suggestion for consistency

The implementation correctly retrieves position and handles errors. For consistency with lines 14 and 21, consider using auto for the error code variable:

-    ErrorCode error_code = try_get_pos(pos);
+    auto error_code = try_get_pos(pos);

src/ystdlib/io_interface/WriterInterface.hpp (2)

14-17: Consider storing the error code in the OperationFailed exception.
Currently, the constructor does not store or utilise the error_code, and no what() override is provided to expose error details.

A possible refactor is to store this code in a private member and override what() to provide diagnostic info.

 class OperationFailed : public std::exception {
 public:
-    OperationFailed(ErrorCode error_code) {}
+    explicit OperationFailed(ErrorCode error_code) : m_error_code(error_code) {}

+    const char* what() const noexcept override {
+        return "OperationFailed in WriterInterface";
+    }

+private:
+    ErrorCode m_error_code;
 };

---

38-39: Ensure endianness is consistent when writing numeric values.
When writing numeric values in binary form, consider documenting or enforcing a specific endianness for interoperability across different platforms.

Also applies to: 71-74

src/ystdlib/io_interface/ReaderInterface.hpp (2)

16-17: Capture and expose the error code in OperationFailed.
Just like in the writer interface, the constructor is empty. Storing the error code and overriding what() can improve debugging.

---

84-94: Recognise potential endianness concerns when reading numeric values.
Similar to the writing interface, reading numeric values in raw binary form can cause portable issues if endianness differs between systems. A documented or enforced endianness may be beneficial.

Also applies to: 126-145

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 3f5186c and 489953f.

📒 Files selected for processing (7)

• src/ystdlib/CMakeLists.txt (1 hunks)
• src/ystdlib/io_interface/CMakeLists.txt (1 hunks)
• src/ystdlib/io_interface/ErrorCode.hpp (1 hunks)
• src/ystdlib/io_interface/ReaderInterface.cpp (1 hunks)
• src/ystdlib/io_interface/ReaderInterface.hpp (1 hunks)
• src/ystdlib/io_interface/WriterInterface.cpp (1 hunks)
• src/ystdlib/io_interface/WriterInterface.hpp (1 hunks)

🧰 Additional context used

📓 Path-based instructions (1)

`**/*.{cpp,hpp,java,js,jsx,tpp,ts,tsx}`: - Prefer `false == ` rather than `!`.

**/*.{cpp,hpp,java,js,jsx,tpp,ts,tsx}: - Prefer false == <expression> rather than !<expression>.

• src/ystdlib/io_interface/ErrorCode.hpp
• src/ystdlib/io_interface/WriterInterface.cpp
• src/ystdlib/io_interface/WriterInterface.hpp
• src/ystdlib/io_interface/ReaderInterface.cpp
• src/ystdlib/io_interface/ReaderInterface.hpp

🧬 Code Definitions (3)

src/ystdlib/io_interface/WriterInterface.cpp (3)

src/ystdlib/io_interface/WriterInterface.hpp (8)

• c (45-45)
• str (50-50)
• pos (30-30)
• pos (32-32)
• pos (56-56)
• OperationFailed (16-16)
• offset (31-31)
• offset (62-62)

src/ystdlib/io_interface/ReaderInterface.cpp (4)

• seek_from_begin (111-116)
• seek_from_begin (111-111)
• get_pos (118-126)
• get_pos (118-118)

src/ystdlib/io_interface/ReaderInterface.hpp (4)

• pos (24-24)
• pos (25-25)
• pos (117-117)
• OperationFailed (16-16)

src/ystdlib/io_interface/ReaderInterface.cpp (1)

src/ystdlib/io_interface/ReaderInterface.hpp (12)

• delim (38-38)
• delim (59-59)
• buf (23-23)
• buf (48-48)
• buf (68-68)
• buf (77-77)
• OperationFailed (16-16)
• str_length (102-102)
• str_length (111-111)
• pos (24-24)
• pos (25-25)
• pos (117-117)

src/ystdlib/io_interface/ReaderInterface.hpp (2)

src/ystdlib/io_interface/WriterInterface.hpp (4)

• pos (30-30)
• pos (32-32)
• pos (56-56)
• OperationFailed (16-16)

src/ystdlib/io_interface/ReaderInterface.cpp (2)

• try_read_exact_length (69-80)
• try_read_exact_length (69-69)

🔇 Additional comments (9)

src/ystdlib/CMakeLists.txt (1)

3-3: LGTM!

The addition of the io_interface module to the build system is clean and follows the project's convention, correctly placed alongside the existing modules.

src/ystdlib/io_interface/CMakeLists.txt (1)

1-14: LGTM!

The CMake configuration is properly structured, clearly defining the public headers, private sources, and test sources for the io_interface library within the ystdlib namespace.

src/ystdlib/io_interface/WriterInterface.cpp (3)

5-7: LGTM!

The implementation correctly delegates to the write method with appropriate parameters.

---

13-18: LGTM!

The implementation follows the project's error handling pattern and correctly adheres to coding guidelines by using ErrorCode_Success != error_code instead of the negated form.

---

20-25: LGTM!

The implementation correctly handles errors and follows the coding guidelines.

src/ystdlib/io_interface/ReaderInterface.cpp (4)

13-15: Good usage of the style guideline.
Using if (false == append) aligns with the coding guideline that prefers false == <expression> over !<expression>.

---

46-55: Consistent error handling approach.
Throwing OperationFailed for non-EOF error codes and returning false for EOF is clear and consistent. Nicely done.

---

69-80: Appropriate handling of truncated reads.
Returning ErrorCode_Truncated when fewer bytes are read helps distinguish partial reads from other errors. This is a good practice for robust error handling.

---

25-29:

✅ Verification successful

Assess partial read handling at EOF.
Returning ErrorCode_Success when EOF is reached but some content was read is a design decision. Ensure that the downstream logic properly differentiates between full and partial reads.

To confirm correctness, please search for call sites of try_read_to_delimiter to confirm all partial-read scenarios are handled correctly:

---

🏁 Script executed:

#!/bin/bash
rg -A 10 "try_read_to_delimiter"

Length of output: 2478

---

Partial read handling at EOF verified

The investigation confirms that the only call site for try_read_to_delimiter (in src/ystdlib/io_interface/ReaderInterface.cpp) correctly distinguishes between a complete EOF (no data read) and a partial read (some data read). When data is read before EOF, the function converts ErrorCode_EndOfFile to ErrorCode_Success, which the caller then treats as a successful read. No further action is required.