From 7d5350c9fb728280a0299dea350d51e85f19d9c4 Mon Sep 17 00:00:00 2001 From: Bingran Hu Date: Fri, 28 Mar 2025 23:05:48 -0400 Subject: [PATCH 1/3] Refactor ReaderInterface --- src/ystdlib/io_interface/ReaderInterface.cpp | 97 ++---------- src/ystdlib/io_interface/ReaderInterface.hpp | 148 ++++++------------- 2 files changed, 62 insertions(+), 183 deletions(-) diff --git a/src/ystdlib/io_interface/ReaderInterface.cpp b/src/ystdlib/io_interface/ReaderInterface.cpp index 381dc98c..425eb7a1 100644 --- a/src/ystdlib/io_interface/ReaderInterface.cpp +++ b/src/ystdlib/io_interface/ReaderInterface.cpp @@ -1,26 +1,25 @@ -// NOLINTBEGIN #include "ReaderInterface.hpp" -using std::string; +#include +#include + +#include "ErrorCode.hpp" namespace ystdlib::io_interface { -ErrorCode ReaderInterface::try_read_to_delimiter( - char delim, - bool keep_delimiter, - bool append, - std::string& str -) { +auto +ReaderInterface::read_to_delimiter(char delim, bool keep_delimiter, bool append, std::string& str) + -> ErrorCode { if (false == append) { str.clear(); } - size_t original_str_length = str.length(); + auto const original_str_length{str.length()}; // Read character by character into str, until we find a delimiter - char c; - size_t num_bytes_read; + char c{0}; + size_t num_bytes_read{0}; while (true) { - auto error_code = try_read(&c, 1, num_bytes_read); + auto const error_code{read(&c, 1, num_bytes_read)}; if (ErrorCode_Success != error_code) { if (ErrorCode_EndOfFile == error_code && str.length() > original_str_length) { return ErrorCode_Success; @@ -43,32 +42,9 @@ ErrorCode ReaderInterface::try_read_to_delimiter( return ErrorCode_Success; } -bool ReaderInterface::read(char* buf, size_t num_bytes_to_read, size_t& num_bytes_read) { - ErrorCode error_code = try_read(buf, num_bytes_to_read, num_bytes_read); - if (ErrorCode_EndOfFile == error_code) { - return false; - } - if (ErrorCode_Success != error_code) { - throw OperationFailed(error_code); - } - return true; -} - -bool ReaderInterface::read_to_delimiter(char delim, bool keep_delimiter, bool append, string& str) { - ErrorCode error_code = try_read_to_delimiter(delim, keep_delimiter, append, str); - if (ErrorCode_EndOfFile == error_code) { - return false; - } - if (ErrorCode_Success != error_code) { - throw OperationFailed(error_code); - } - - return true; -} - -ErrorCode ReaderInterface::try_read_exact_length(char* buf, size_t num_bytes) { - size_t num_bytes_read; - auto error_code = try_read(buf, num_bytes, num_bytes_read); +auto ReaderInterface::read_exact_length(char* buf, size_t num_bytes) -> ErrorCode { + size_t num_bytes_read{0}; + auto const error_code{read(buf, num_bytes, num_bytes_read)}; if (ErrorCode_Success != error_code) { return error_code; } @@ -79,51 +55,10 @@ ErrorCode ReaderInterface::try_read_exact_length(char* buf, size_t num_bytes) { return ErrorCode_Success; } -bool ReaderInterface::read_exact_length(char* buf, size_t num_bytes, bool eof_possible) { - ErrorCode error_code = try_read_exact_length(buf, num_bytes); - if (eof_possible && ErrorCode_EndOfFile == error_code) { - return false; - } - if (ErrorCode_Success != error_code) { - throw OperationFailed(error_code); - } - return true; -} - -ErrorCode ReaderInterface::try_read_string(size_t const str_length, string& str) { +auto ReaderInterface::read_string(size_t const str_length, std::string& str) -> ErrorCode { // Resize string to fit str_length str.resize(str_length); - return try_read_exact_length(&str[0], str_length); -} - -bool ReaderInterface::read_string(size_t const str_length, string& str, bool eof_possible) { - ErrorCode error_code = try_read_string(str_length, str); - if (eof_possible && ErrorCode_EndOfFile == error_code) { - return false; - } - if (ErrorCode_Success != error_code) { - throw OperationFailed(error_code); - } - return true; -} - -void ReaderInterface::seek_from_begin(size_t pos) { - ErrorCode error_code = try_seek_from_begin(pos); - if (ErrorCode_Success != error_code) { - throw OperationFailed(error_code); - } -} - -size_t ReaderInterface::get_pos() { - size_t pos; - ErrorCode error_code = try_get_pos(pos); - if (ErrorCode_Success != error_code) { - throw OperationFailed(error_code); - } - - return pos; + return read_exact_length(str.data(), str_length); } } // namespace ystdlib::io_interface - -// NOLINTEND diff --git a/src/ystdlib/io_interface/ReaderInterface.hpp b/src/ystdlib/io_interface/ReaderInterface.hpp index dde1a503..84652f64 100644 --- a/src/ystdlib/io_interface/ReaderInterface.hpp +++ b/src/ystdlib/io_interface/ReaderInterface.hpp @@ -1,6 +1,7 @@ #ifndef YSTDLIB_IO_INTERFACE_READERINTERFACE_HPP #define YSTDLIB_IO_INTERFACE_READERINTERFACE_HPP -// NOLINTBEGIN + +#include #include #include @@ -10,140 +11,83 @@ namespace ystdlib::io_interface { class ReaderInterface { public: - // Types - class OperationFailed : public std::exception { - public: - OperationFailed(ErrorCode error_code) {} - }; + // Constructor + ReaderInterface() = default; - // Destructor - virtual ~ReaderInterface() = default; + // Delete copy constructor and assignment operator + ReaderInterface(ReaderInterface const&) = delete; + auto operator=(ReaderInterface const&) -> ReaderInterface& = delete; - // Methods - virtual ErrorCode try_read(char* buf, size_t num_bytes_to_read, size_t& num_bytes_read) = 0; - virtual ErrorCode try_seek_from_begin(size_t pos) = 0; - virtual ErrorCode try_get_pos(size_t& pos) = 0; + // Default move constructor and assignment operator + ReaderInterface(ReaderInterface&&) noexcept = default; + auto operator=(ReaderInterface&&) noexcept -> ReaderInterface& = default; - /** - * Tries to read up to the next delimiter and stores it in the given string. - * NOTE: Implementations should override this if they can achieve better performance. - * @param delim The delimiter to stop at - * @param keep_delimiter Whether to include the delimiter in the output string or not - * @param append Whether to append to the given string or replace its contents - * @param str The string read - * @return ErrorCode_Success on success - * @return Same as ReaderInterface::try_read otherwise - */ - virtual ErrorCode - try_read_to_delimiter(char delim, bool keep_delimiter, bool append, std::string& str); + // Destructor + virtual ~ReaderInterface() = default; - /** - * Reads up to a given number of bytes + // Methods implementing ReaderInterface + /* + * Reads up to the given number of bytes from the underlying medium into the given buffer. * @param buf - * @param num_bytes_to_read The number of bytes to try and read - * @param num_bytes_read The actual number of bytes read - * @return false on EOF - * @return true otherwise + * @param num_bytes_to_read + * @param num_bytes_read Returns the actual number of bytes read. */ - bool read(char* buf, size_t num_bytes_to_read, size_t& num_bytes_read); + [[nodiscard]] virtual auto read(char* buf, size_t num_bytes_to_read, size_t& num_bytes_read) + -> ErrorCode + = 0; /** - * Reads up to the next delimiter and stores it in the given string - * @param delim The delimiter to stop at - * @param keep_delimiter Whether to include the delimiter in the output string or not - * @param append Whether to append to the given string or replace its contents - * @param str The string read - * @return false on EOF - * @return true on success + * Reads up to the next delimiter from the underlying medium into the given string. + * @param delim The delimiter to stop at. + * @param keep_delimiter Whether to include the delimiter in the output string or not. + * @param append Whether to append to the given string or replace its contents. + * @param str Returns the string read. */ - bool read_to_delimiter(char delim, bool keep_delimiter, bool append, std::string& str); + [[nodiscard]] virtual auto + read_to_delimiter(char delim, bool keep_delimiter, bool append, std::string& str) -> ErrorCode; /** - * Tries to read a number of bytes - * @param buf - * @param num_bytes Number of bytes to read - * @return Same as the underlying medium's try_read method - * @return ErrorCode_Truncated if 0 < # bytes read < num_bytes - */ - ErrorCode try_read_exact_length(char* buf, size_t num_bytes); - /** - * Reads a number of bytes + * Reads the given number of bytes from the underlying medium into the given buffer. * @param buf - * @param num_bytes Number of bytes to read - * @param eof_possible If EOF should be possible (without reading any bytes) - * @return false if EOF is possible and EOF was hit - * @return true on success + * @param num_bytes Number of bytes to read. */ - bool read_exact_length(char* buf, size_t num_bytes, bool eof_possible); + [[nodiscard]] virtual auto read_exact_length(char* buf, size_t num_bytes) -> ErrorCode; /** - * Tries to read a numeric value from a file - * @param value The read value - * @return Same as FileReader::try_read_exact_length's return values + * @param value Returns the read numeric value. */ template - ErrorCode try_read_numeric_value(ValueType& value); - /** - * Reads a numeric value - * @param value The read value - * @param eof_possible If EOF should be possible (without reading any bytes) - * @return false if EOF is possible and EOF was hit - * @return true on success - */ - template - bool read_numeric_value(ValueType& value, bool eof_possible); + [[nodiscard]] auto read_numeric_value(ValueType& value) -> ErrorCode; /** - * Tries to read a string * @param str_length - * @param str The string read - * @return Same as ReaderInterface::try_read_exact_length + * @param str Returns the string read. */ - ErrorCode try_read_string(size_t str_length, std::string& str); + [[nodiscard]] virtual auto read_string(size_t str_length, std::string& str) -> ErrorCode; + + // Methods implementing general stream interface /** - * Reads a string - * @param str_length - * @param str The string read - * @param eof_possible If EOF should be possible (without reading any bytes) - * @return false if EOF is possible and EOF was hit - * @return true on success + * Seeks from the beginning to the given position. + * @param pos */ - bool read_string(size_t str_length, std::string& str, bool eof_possible); + [[nodiscard]] virtual auto seek_from_begin(size_t pos) -> ErrorCode = 0; /** - * Seeks from the beginning to the given position - * @param pos + * Seeks from the current position to the next position by the given offset amount. + * @param offset */ - void seek_from_begin(size_t pos); + [[nodiscard]] virtual auto seek_from_current(off_t offset) -> ErrorCode = 0; /** - * Gets the current position of the read head - * @return Position of the read head + * @param pos Returns the current position of the stream pointer. */ - size_t get_pos(); + [[nodiscard]] virtual auto get_pos(size_t& pos) -> ErrorCode = 0; }; template -ErrorCode ReaderInterface::try_read_numeric_value(ValueType& value) { - ErrorCode error_code = try_read_exact_length(reinterpret_cast(&value), sizeof(value)); - if (ErrorCode_Success != error_code) { - return error_code; - } - return ErrorCode_Success; -} - -template -bool ReaderInterface::read_numeric_value(ValueType& value, bool eof_possible) { - ErrorCode error_code = try_read_numeric_value(value); - if (ErrorCode_EndOfFile == error_code && eof_possible) { - return false; - } - if (ErrorCode_Success != error_code) { - throw OperationFailed(error_code); - } - return true; +auto ReaderInterface::read_numeric_value(ValueType& value) -> ErrorCode { + return read_exact_length(static_cast(&value), sizeof(ValueType)); } } // namespace ystdlib::io_interface -// NOLINTEND #endif // YSTDLIB_IO_INTERFACE_READERINTERFACE_HPP From b8d41db99b78b16a224d5bc7930659bb804db6c3 Mon Sep 17 00:00:00 2001 From: Bingran Hu Date: Mon, 31 Mar 2025 11:13:31 -0400 Subject: [PATCH 2/3] Silence warnings --- src/ystdlib/io_interface/ReaderInterface.hpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/ystdlib/io_interface/ReaderInterface.hpp b/src/ystdlib/io_interface/ReaderInterface.hpp index 84652f64..09782ff9 100644 --- a/src/ystdlib/io_interface/ReaderInterface.hpp +++ b/src/ystdlib/io_interface/ReaderInterface.hpp @@ -1,6 +1,8 @@ #ifndef YSTDLIB_IO_INTERFACE_READERINTERFACE_HPP #define YSTDLIB_IO_INTERFACE_READERINTERFACE_HPP +// TODO: https://github.com/y-scope/ystdlib-cpp/issues/50 +// NOLINTNEXTLINE(misc-include-cleaner) #include #include @@ -76,6 +78,8 @@ class ReaderInterface { * Seeks from the current position to the next position by the given offset amount. * @param offset */ + // TODO: https://github.com/y-scope/ystdlib-cpp/issues/50 + // NOLINTNEXTLINE(misc-include-cleaner) [[nodiscard]] virtual auto seek_from_current(off_t offset) -> ErrorCode = 0; /** From b86ee54ee062c01f2ad9dc0315387885feca1819 Mon Sep 17 00:00:00 2001 From: Bingran Hu Date: Mon, 31 Mar 2025 19:12:28 -0400 Subject: [PATCH 3/3] Apply suggestions from code review Co-authored-by: davidlion --- src/ystdlib/io_interface/ReaderInterface.hpp | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/ystdlib/io_interface/ReaderInterface.hpp b/src/ystdlib/io_interface/ReaderInterface.hpp index 09782ff9..bf68d5f8 100644 --- a/src/ystdlib/io_interface/ReaderInterface.hpp +++ b/src/ystdlib/io_interface/ReaderInterface.hpp @@ -27,7 +27,7 @@ class ReaderInterface { // Destructor virtual ~ReaderInterface() = default; - // Methods implementing ReaderInterface + // Methods /* * Reads up to the given number of bytes from the underlying medium into the given buffer. * @param buf @@ -67,7 +67,6 @@ class ReaderInterface { */ [[nodiscard]] virtual auto read_string(size_t str_length, std::string& str) -> ErrorCode; - // Methods implementing general stream interface /** * Seeks from the beginning to the given position. * @param pos @@ -83,7 +82,7 @@ class ReaderInterface { [[nodiscard]] virtual auto seek_from_current(off_t offset) -> ErrorCode = 0; /** - * @param pos Returns the current position of the stream pointer. + * @param pos Returns the current position of the read pointer. */ [[nodiscard]] virtual auto get_pos(size_t& pos) -> ErrorCode = 0; };