Refactor code to address clang-tidy warnings

src/limestone/compaction_options.h

@@ -26,98 +26,97 @@
  namespace limestone::internal {
 
  class compaction_options {
- public:
+  public:
      // Constructor: Pre-compaction phase where to_dir is not provided.
      // File set is provided and GC is disabled.
      // In this constructor, "/not_exists_dir" is set for to_dir to indicate an error if used.
      compaction_options(
-         const boost::filesystem::path& from,
+         boost::filesystem::path from,
          int workers,
          std::set<std::string> file_names
      )
-     : from_dir_(from),
+     : from_dir_(std::move(from)),
        to_dir_("/not_exists_dir"),
        num_worker_(workers),
-       file_names_(file_names),
+       file_names_(std::move(file_names)),
        has_file_set_(true),
        gc_snapshot_(std::nullopt)
      {}
 
      // Constructor: to_dir provided, GC disabled, no file set.
      compaction_options(
-         const boost::filesystem::path& from,
-         const boost::filesystem::path& to,
+         boost::filesystem::path from,
+         boost::filesystem::path to,
          int workers
      )
-     : from_dir_(from),
-       to_dir_(to),
+     : from_dir_(std::move(from)),
+       to_dir_(std::move(to)),
        num_worker_(workers),
-       file_names_(),
        has_file_set_(false),
        gc_snapshot_(std::nullopt)
      {}
 
      // Constructor: to_dir provided, file set available, GC disabled.
      compaction_options(
-         const boost::filesystem::path& from,
-         const boost::filesystem::path& to,
+         boost::filesystem::path from,
+         boost::filesystem::path to,
          int workers,
          std::set<std::string> file_names
      )
-     : from_dir_(from),
-       to_dir_(to),
+     : from_dir_(std::move(from)),
+       to_dir_(std::move(to)),
        num_worker_(workers),
-       file_names_(file_names),
+       file_names_(std::move(file_names)),
        has_file_set_(true),
        gc_snapshot_(std::nullopt)
      {}
 
      // Constructor: to_dir provided, file set available, GC enabled.
      // Note: The blob_file_gc_snapshot is provided externally by reference.
      compaction_options(
-         const boost::filesystem::path& from,
-         const boost::filesystem::path& to,
+         boost::filesystem::path from,
+         boost::filesystem::path to,
          int workers,
          std::set<std::string> file_names,
          blob_file_gc_snapshot& gc_snapshot
      )
-     : from_dir_(from),
-       to_dir_(to),
+     : from_dir_(std::move(from)),
+       to_dir_(std::move(to)),
        num_worker_(workers),
-       file_names_(file_names),
+       file_names_(std::move(file_names)),
        has_file_set_(true),
        gc_snapshot_(std::ref(gc_snapshot))
      {}
 
-     // Getter for from_dir
-     const boost::filesystem::path& get_from_dir() const { return from_dir_; }
+     // Getter for from_dir.
+     [[nodiscard]] const boost::filesystem::path& get_from_dir() const { return from_dir_; }
 
      // Getter for to_dir.
-     const boost::filesystem::path& get_to_dir() const { return to_dir_; }
+     [[nodiscard]] const boost::filesystem::path& get_to_dir() const { return to_dir_; }
 
      // Getter for num_worker.
-     int get_num_worker() const { return num_worker_; }
+     [[nodiscard]] int get_num_worker() const { return num_worker_; }
 
      // Getter for file_names.
-     const std::set<std::string>& get_file_names() const { return file_names_; }
+     [[nodiscard]] const std::set<std::string>& get_file_names() const { return file_names_; }
 
      // Returns true if a file set is configured.
-     bool has_file_set() const { return has_file_set_; }
+     [[nodiscard]] bool has_file_set() const { return has_file_set_; }
 
      // Check if GC is enabled.
-     bool is_gc_enabled() const { return gc_snapshot_.has_value(); }
+     [[nodiscard]] bool is_gc_enabled() const { return gc_snapshot_.has_value(); }
 
      // Getter for gc_snapshot.
      // It is caller's responsibility to ensure GC is enabled before calling.
-     blob_file_gc_snapshot& get_gc_snapshot() const { return gc_snapshot_.value().get(); }
+     [[nodiscard]] blob_file_gc_snapshot& get_gc_snapshot() const { return gc_snapshot_.value().get(); }
 
- private:
+  private:
      // Basic compaction settings.
      boost::filesystem::path from_dir_;
      boost::filesystem::path to_dir_;
      int num_worker_;
 
-    // File set for compaction
+     // File set for compaction.
      std::set<std::string> file_names_;
      bool has_file_set_;
 

src/limestone/datastore_snapshot.cpp

@@ -109,7 +109,7 @@ static void insert_twisted_entry(sortdb_wrapper* sortdb, const log_entry& e) {
     sortdb->put(db_key, db_value);
 }
 
-static std::pair<epoch_id_type, sorting_context> create_sorted_from_wals(compaction_options options) {
+static std::pair<epoch_id_type, sorting_context> create_sorted_from_wals(compaction_options &options) {
     auto from_dir = options.get_from_dir();
     auto file_names = options.get_file_names();
     auto num_worker = options.get_num_worker();
@@ -300,11 +300,11 @@ static void sortdb_foreach(
 #endif
 }
 
-blob_id_type create_compact_pwal_and_get_max_blob_id(compaction_options options) {
+blob_id_type create_compact_pwal_and_get_max_blob_id(compaction_options &options) {
     auto [max_appeared_epoch, sctx] = create_sorted_from_wals(options);
 
     boost::system::error_code error;
-    auto to_dir = options.get_to_dir();
+    const auto &to_dir = options.get_to_dir();
     const bool result_check = boost::filesystem::exists(to_dir, error);
     if (!result_check || error) {
         const bool result_mkdir = boost::filesystem::create_directory(to_dir, error);

src/limestone/dblog_scan.h

@@ -82,8 +82,10 @@ class dblog_scan {
      *
      * @param logdir The path to the directory containing the files to be processed.
      */
+     explicit dblog_scan(boost::filesystem::path logdir) : dblogdir_(std::move(logdir)) {
+        rescan_directory_paths();
+    }
 
-    explicit dblog_scan(const boost::filesystem::path& logdir) : dblogdir_(logdir) { rescan_directory_paths(); }
 
     /**
      * @brief Constructor that initializes the dblog_scan with the specified log directory and file names.
@@ -95,8 +97,8 @@ class dblog_scan {
      * @param logdir The path to the directory containing the files to be processed.
      * @param file_names The set of file names within `logdir` to be processed.
      */
-    explicit dblog_scan(const boost::filesystem::path& logdir, compaction_options options) : dblogdir_(logdir) {
-        for (const auto& file_name : options.get_file_names()) {
+    explicit dblog_scan(boost::filesystem::path logdir, const compaction_options &options) : dblogdir_(std::move(logdir)) {
+        for (const auto &file_name : options.get_file_names()) {
             path_list_.emplace_back(dblogdir_ / file_name);
         }
     }

src/limestone/internal.h

@@ -95,7 +95,7 @@ status purge_dir(const boost::filesystem::path& dir);
  *                number of workers, file set, and garbage collection settings.
  * @return The maximum blob ID found during the compaction process.
  */
-limestone::api::blob_id_type create_compact_pwal_and_get_max_blob_id(compaction_options options);
+limestone::api::blob_id_type create_compact_pwal_and_get_max_blob_id(compaction_options &options);
 
 
 std::set<boost::filesystem::path> filter_epoch_files(const boost::filesystem::path& directory);

test/limestone/compaction/compaction_options_test.cpp

@@ -70,7 +70,7 @@
      compaction_options options(from_dir_, to_dir_, num_workers_);
 
      EXPECT_FALSE(options.is_gc_enabled());
-     EXPECT_THROW(options.get_gc_snapshot(), std::bad_optional_access);
+     EXPECT_THROW((void)options.get_gc_snapshot(), std::bad_optional_access);
  }
 
  // New test for the constructor without to_dir (pre-compaction phase)