Apply suggestions from code review

Co-authored-by: kirkrodrigues <[email protected]>
y-scope · Jun 7, 2024 · 83b5385 · 83b5385
1 parent 8674f6a
commit 83b5385
Show file tree

Hide file tree

Showing 3 changed files with 19 additions and 16 deletions.
diff --git a/components/core/src/clp/clp/FileDecompressor.cpp b/components/core/src/clp/clp/FileDecompressor.cpp
@@ -15,9 +15,10 @@ using std::string;
 namespace clp::clp {
 namespace {
 /**
- * Renames a temporary IR and moves it to the output directory.
- * The new name follows the following format:
- * <FILE_ORIG_ID>_<begin_message_ix>_<end_message_ix>.clp.zst
+ * Renames a temporary IR file and moves it to the output directory.
+ *
+ * The new name uses the following format:
+ * <orig_file_id>_<begin_message_ix>_<end_message_ix>.clp.zst
  * @param temp_ir
  * @param output_directory
  * @param file_orig_id
@@ -28,11 +29,11 @@ namespace {
 bool rename_ir_file(
         boost::filesystem::path const& temp_ir_path,
         boost::filesystem::path const& output_directory,
-        std::string const& file_orig_id,
+        string const& file_orig_id,
         size_t begin_message_ix,
         size_t end_message_ix
 ) {
-    std::string ir_file_name = file_orig_id + "_" + std::to_string(begin_message_ix) + "_"
+    string ir_file_name = file_orig_id + "_" + std::to_string(begin_message_ix) + "_"
                                + std::to_string(end_message_ix) + ir::cIrExtension;
 
     auto const renamed_ir_path = output_directory / ir_file_name;
@@ -187,6 +188,7 @@ bool FileDecompressor::decompress_to_ir(
             SPDLOG_ERROR("Failed to decompress message");
             return false;
         }
+
         if (ir_serializer.get_serialized_size() >= ir_target_size) {
             ir_serializer.close();
 
@@ -228,14 +230,14 @@ bool FileDecompressor::decompress_to_ir(
     auto const end_message_ix = begin_message_ix + ir_serializer.get_num_log_events();
     ir_serializer.close();
 
-    // Note we don't remove the temp_output_dir because we don't know if it exists before execution
+    // NOTE: We don't remove temp_output_dir because we don't know if it existed before this method
+    // was called.
     if (false
         == rename_ir_file(temp_ir_path, output_dir, file_orig_id, begin_message_ix, end_message_ix))
     {
         return false;
     }
 
-    // Close files
     archive_reader.close_file(m_encoded_file);
     return true;
 }

diff --git a/components/core/src/clp/ir/LogEventSerializer.cpp b/components/core/src/clp/ir/LogEventSerializer.cpp
@@ -5,6 +5,7 @@
 
 #include <spdlog/spdlog.h>
 
+#include "../Defs.h"
 #include "../ErrorCode.hpp"
 #include "../ffi/ir_stream/encoding_methods.hpp"
 #include "../ffi/ir_stream/protocol_constants.hpp"
@@ -18,7 +19,8 @@ namespace clp::ir {
 template <typename encoded_variable_t>
 LogEventSerializer<encoded_variable_t>::~LogEventSerializer() {
     if (m_is_open) {
-        SPDLOG_ERROR("Serializer is not closed before being destroyed - output maybe corrupted");
+        SPDLOG_ERROR("clp::ir::LogEventSerializer not closed before being destroyed - output maybe "
+                     "corrupted.");
     }
 }
 
@@ -38,7 +40,7 @@ auto LogEventSerializer<encoded_variable_t>::open(string const& file_path) -> Er
     bool res{};
     if constexpr (std::is_same_v<encoded_variable_t, four_byte_encoded_variable_t>) {
         m_prev_msg_timestamp = 0;
-        res = clp::ffi::ir_stream::four_byte_encoding::serialize_preamble(
+        res = ffi::ir_stream::four_byte_encoding::serialize_preamble(
                 cTimestampPattern,
                 cTimestampPatternSyntax,
                 cTimezoneID,
@@ -55,7 +57,6 @@ auto LogEventSerializer<encoded_variable_t>::open(string const& file_path) -> Er
     }
 
     if (false == res) {
-        SPDLOG_ERROR("Failed to serialize preamble");
         close_writer();
         return ErrorCode_Failure;
     }
@@ -141,10 +142,10 @@ template auto LogEventSerializer<eight_byte_encoded_variable_t>::open(string con
 ) -> ErrorCode;
 template auto LogEventSerializer<four_byte_encoded_variable_t>::open(string const& file_path
 ) -> ErrorCode;
-template auto LogEventSerializer<four_byte_encoded_variable_t>::flush() -> void;
 template auto LogEventSerializer<eight_byte_encoded_variable_t>::flush() -> void;
-template auto LogEventSerializer<four_byte_encoded_variable_t>::close() -> void;
+template auto LogEventSerializer<four_byte_encoded_variable_t>::flush() -> void;
 template auto LogEventSerializer<eight_byte_encoded_variable_t>::close() -> void;
+template auto LogEventSerializer<four_byte_encoded_variable_t>::close() -> void;
 template auto LogEventSerializer<eight_byte_encoded_variable_t>::serialize_log_event(
         string_view message,
         epoch_time_ms_t timestamp
@@ -153,6 +154,6 @@ template auto LogEventSerializer<four_byte_encoded_variable_t>::serialize_log_ev
         string_view message,
         epoch_time_ms_t timestamp
 ) -> ErrorCode;
-template auto LogEventSerializer<four_byte_encoded_variable_t>::close_writer() -> void;
 template auto LogEventSerializer<eight_byte_encoded_variable_t>::close_writer() -> void;
+template auto LogEventSerializer<four_byte_encoded_variable_t>::close_writer() -> void;
 }  // namespace clp::ir
diff --git a/components/core/src/clp/ir/LogEventSerializer.hpp b/components/core/src/clp/ir/LogEventSerializer.hpp
@@ -17,9 +17,9 @@
 
 namespace clp::ir {
 /**
- * Class for serializing log events into a Zstandard compressed IR stream. The serializer first
- * buffers the serialized data into an internal buffer, and only flushes the buffered ir into the
- * on-disk file when `flush` or `close` is called.
+ * Class for serializing log events into a Zstandard-compressed IR stream. The serializer first
+ * buffers the serialized data into an internal buffer, and only flushes the buffered IR to disk
+ * when `flush` or `close` is called.
  */
 template <typename encoded_variable_t>
 class LogEventSerializer {