refactor(io_interface): Change ReaderInterface to return ErrorCodes instead of throwing exceptions; address clang-tidy violations in ReaderInterface.
#49
Conversation
WalkthroughThe pull request refactors the Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Reader
Client->>Reader: read_string(str_length, str)
Reader->>Reader: read_exact_length(buf, num_bytes)
Reader-->>Client: ErrorCode
sequenceDiagram
participant Client
participant Reader
Client->>Reader: read_to_delimiter(delim, keep_delimiter, append, str)
Reader-->>Client: ErrorCode
Possibly related PRs
Suggested reviewers
✨ Finishing Touches
🪧 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 (
|
Bill-hbrhbr
changed the title
OperationFailed and have all functions return ErrorCode in the ReaderInterface class.ReaderInterface error handling using ErrorCode and remove exception throwing; run clang-tidy on changed files.
Co-authored-by: davidlion <[email protected]>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
src/ystdlib/io_interface/ReaderInterface.hpp (3)
4-6: TODO and NOLINTNEXTLINE present despite PR objective to remove themThe PR objectives mentioned removing all
NOLINTcomments, but there's still aNOLINTNEXTLINEcomment here. Consider addressing the TODO item in a separate PR or updating the current PR's objectives to clarify that some TODOs and NOLINT comments will remain.
48-49: Method formatting inconsistencyThe
read_to_delimitermethod declaration spans two lines with a different formatting style than other methods. Consider making the formatting consistent across all methods.- [[nodiscard]] virtual auto - read_to_delimiter(char delim, bool keep_delimiter, bool append, std::string& str) -> ErrorCode; + [[nodiscard]] virtual auto read_to_delimiter(char delim, bool keep_delimiter, bool append, std::string& str) + -> ErrorCode;
80-82: TODO and NOLINTNEXTLINE still presentSimilar to the earlier TODO, there's another one here with a NOLINTNEXTLINE comment. Consider addressing this in a separate PR or updating the current PR objectives.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/ystdlib/io_interface/ReaderInterface.hpp(2 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}: - Preferfalse == <expression>rather than!<expression>.
src/ystdlib/io_interface/ReaderInterface.hpp
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: lint-check (macos-15)
- GitHub Check: lint-check (ubuntu-24.04)
🔇 Additional comments (8)
src/ystdlib/io_interface/ReaderInterface.hpp (8)
16-25: Good addition of special member functions with proper semanticsThe class now properly defines its special member functions following the rule of five. Deleting copy operations and enabling move operations is a good practice for resource-owning interfaces, especially ones that might be used in containers or transferred between objects.
31-39: Improved method signature with[[nodiscard]]and error code returnThe method signature change from returning
boolto returningErrorCodewith the[[nodiscard]]attribute aligns well with the PR objective of enhancing error handling. This ensures errors aren't accidentally ignored.
56-56: Good removal ofeof_possibleparameterThe removal of the
eof_possibleparameter fromread_exact_lengthaligns with the PR objective of eliminating these flags in favor of returning EOF-related error codes directly.
62-62: Good template method alignment with error code approachThe
read_numeric_valuemethod now correctly returns anErrorCodeand includes the[[nodiscard]]attribute, consistent with the pattern applied to other methods in the class.
68-68: Good removal ofeof_possibleparameter fromread_stringSimilar to
read_exact_length, removing theeof_possibleparameter fromread_stringaligns with the PR objective of returning EOF-related error codes directly.
74-74: Trailing return type forseek_from_beginThe signature change for
seek_from_beginto returnErrorCodeis good, consistent with the other methods.
87-87: Good method signature update forget_posChanging
get_posto return anErrorCodeand take a reference parameter for the position aligns well with the error handling approach. This provides a clear way to handle errors when retrieving the current position.
91-92: Template method implementation now uses error codeThe implementation of
read_numeric_valuenow correctly uses and returns theErrorCodefromread_exact_length. This is a good change that aligns with the overall error handling approach.
Bill-hbrhbr
changed the title
ReaderInterface error handling using ErrorCode and remove exception throwing; run clang-tidy on changed files.ReaderInterface to return ErrorCodes instead of throwing exceptions; remove NOLINTs and fix clang-tidy issues.
Bill-hbrhbr
changed the title
ReaderInterface to return ErrorCodes instead of throwing exceptions; remove NOLINTs and fix clang-tidy issues.ReaderInterface to return ErrorCodes instead of throwing exceptions; remove ReaderInterface NOLINTs and fix clang-tidy issues.
davidlion
changed the title
ReaderInterface to return ErrorCodes instead of throwing exceptions; remove ReaderInterface NOLINTs and fix clang-tidy issues.ReaderInterface to return ErrorCodes instead of throwing exceptions; address clang-tidy violations in ReaderInterface.
davidlion
changed the title
ReaderInterface to return ErrorCodes instead of throwing exceptions; address clang-tidy violations in ReaderInterface.ReaderInterface to return ErrorCodes instead of throwing exceptions; address clang-tidy violations in ReaderInterface.
2 participants
Description
Consolidate
try_do_something()anddo_something()functions into a single implementation in the API.OperationFailedand all exception handlings, since users should interact withystdlib-cppexclusively via theErrorCodesystem.ErrorCodeinstead of throwing exceptions.eof_possibleflags from applicable functions and return EOF-related error codes directly.Remove all
NOLINTcomments and applyclang-tidyfixes on the refactored files, except for Provide a well-definedoff_ttype across different operating system platforms. #50Checklist
breaking change.
Validation performed
Summary by CodeRabbit
Summary by CodeRabbit
[[nodiscard]]attributes.