Skip to content

Commit

Permalink
Refactor code to address clang-tidy warnings
Browse files Browse the repository at this point in the history
  • Loading branch information
umegane committed Feb 18, 2025
1 parent 73ea9e7 commit 31ffbc8
Show file tree
Hide file tree
Showing 2 changed files with 22 additions and 23 deletions.
43 changes: 21 additions & 22 deletions src/limestone/compaction_options.h
Original file line number Diff line number Diff line change
Expand Up @@ -26,70 +26,69 @@
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
// Getter for from_dir.
const boost::filesystem::path& get_from_dir() const { return from_dir_; }

Check warning on line 92 in src/limestone/compaction_options.h

View workflow job for this annotation

GitHub Actions / Clang-Tidy

modernize-use-nodiscard

function 'get_from_dir' should be marked [[nodiscard]]

// Getter for to_dir.
Expand All @@ -111,13 +110,13 @@
// 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(); }

Check warning on line 111 in src/limestone/compaction_options.h

View workflow job for this annotation

GitHub Actions / Clang-Tidy

modernize-use-nodiscard

function 'get_gc_snapshot' should be marked [[nodiscard]]

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_;

Expand Down
2 changes: 1 addition & 1 deletion test/limestone/compaction/compaction_options_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down

0 comments on commit 31ffbc8

Please sign in to comment.