Skip to content

Commit 7e99399

Browse files
HeartLinkedHeartLinked
authored
refactor: make ConfigBase subclass constructors private (apache#368)
1 parent 9805fae commit 7e99399

File tree

7 files changed

+53
-37
lines changed

7 files changed

+53
-37
lines changed

‎src/iceberg/file_reader.cc‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -60,12 +60,12 @@ Result<std::unique_ptr<Reader>> ReaderFactoryRegistry::Open(
6060
}
6161

6262
std::unique_ptr<ReaderProperties> ReaderProperties::default_properties() {
63-
return std::make_unique<ReaderProperties>();
63+
return std::unique_ptr<ReaderProperties>(new ReaderProperties());
6464
}
6565

6666
std::unique_ptr<ReaderProperties> ReaderProperties::FromMap(
6767
const std::unordered_map<std::string, std::string>& properties) {
68-
auto reader_properties = std::make_unique<ReaderProperties>();
68+
auto reader_properties = std::unique_ptr<ReaderProperties>(new ReaderProperties());
6969
reader_properties->configs_ = properties;
7070
return reader_properties;
7171
}

‎src/iceberg/file_reader.h‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,9 @@ class ReaderProperties : public ConfigBase<ReaderProperties> {
8282
/// \brief Create a ReaderProperties instance from a map of key-value pairs.
8383
static std::unique_ptr<ReaderProperties> FromMap(
8484
const std::unordered_map<std::string, std::string>& properties);
85+
86+
private:
87+
ReaderProperties() = default;
8588
};
8689

8790
/// \brief Options for creating a reader.

‎src/iceberg/file_writer.cc‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -60,12 +60,12 @@ Result<std::unique_ptr<Writer>> WriterFactoryRegistry::Open(
6060
}
6161

6262
std::unique_ptr<WriterProperties> WriterProperties::default_properties() {
63-
return std::make_unique<WriterProperties>();
63+
return std::unique_ptr<WriterProperties>(new WriterProperties());
6464
}
6565

6666
std::unique_ptr<WriterProperties> WriterProperties::FromMap(
6767
const std::unordered_map<std::string, std::string>& properties) {
68-
auto writer_properties = std::make_unique<WriterProperties>();
68+
auto writer_properties = std::unique_ptr<WriterProperties>(new WriterProperties());
6969
writer_properties->configs_ = properties;
7070
return writer_properties;
7171
}

‎src/iceberg/file_writer.h‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,9 @@ class WriterProperties : public ConfigBase<WriterProperties> {
5757
/// \brief Create a WriterProperties instance from a map of key-value pairs.
5858
static std::unique_ptr<WriterProperties> FromMap(
5959
const std::unordered_map<std::string, std::string>& properties);
60+
61+
private:
62+
WriterProperties() = default;
6063
};
6164

6265
/// \brief Options for creating a writer.

‎src/iceberg/table_properties.cc‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,12 +32,12 @@ const std::unordered_set<std::string>& TableProperties::reserved_properties() {
3232
}
3333

3434
std::unique_ptr<TableProperties> TableProperties::default_properties() {
35-
return std::make_unique<TableProperties>();
35+
return std::unique_ptr<TableProperties>(new TableProperties());
3636
}
3737

3838
std::unique_ptr<TableProperties> TableProperties::FromMap(
3939
const std::unordered_map<std::string, std::string>& properties) {
40-
auto table_properties = std::make_unique<TableProperties>();
40+
auto table_properties = std::unique_ptr<TableProperties>(new TableProperties());
4141
for (const auto& [key, value] : properties) { // NOLINT(modernize-type-traits)
4242
table_properties->configs_[key] = value;
4343
}

‎src/iceberg/table_properties.h‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -295,6 +295,9 @@ class ICEBERG_EXPORT TableProperties : public ConfigBase<TableProperties> {
295295
/// \return A unique pointer to a TableProperties instance
296296
static std::unique_ptr<TableProperties> FromMap(
297297
const std::unordered_map<std::string, std::string>& properties);
298+
299+
private:
300+
TableProperties() = default;
298301
};
299302

300303
} // namespace iceberg

‎src/iceberg/test/config_test.cc‎

Lines changed: 38 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -64,48 +64,55 @@ class TestConfig : public ConfigBase<TestConfig> {
6464
inline static const Entry<TestEnum> kEnumConfig{"enum_config", TestEnum::VALUE1,
6565
EnumToString, StringToEnum};
6666
inline static const Entry<double> kDoubleConfig{"double_config", 3.14};
67+
68+
static std::unique_ptr<TestConfig> default_properties() {
69+
return std::unique_ptr<TestConfig>(new TestConfig());
70+
}
71+
72+
private:
73+
TestConfig() = default;
6774
};
6875

6976
TEST(ConfigTest, BasicOperations) {
70-
TestConfig config;
77+
auto config = TestConfig::default_properties();
7178

7279
// Test default values
73-
ASSERT_EQ(config.Get(TestConfig::kStringConfig), std::string("default_value"));
74-
ASSERT_EQ(config.Get(TestConfig::kIntConfig), 25);
75-
ASSERT_EQ(config.Get(TestConfig::kBoolConfig), false);
76-
ASSERT_EQ(config.Get(TestConfig::kEnumConfig), TestEnum::VALUE1);
77-
ASSERT_EQ(config.Get(TestConfig::kDoubleConfig), 3.14);
80+
ASSERT_EQ(config->Get(TestConfig::kStringConfig), std::string("default_value"));
81+
ASSERT_EQ(config->Get(TestConfig::kIntConfig), 25);
82+
ASSERT_EQ(config->Get(TestConfig::kBoolConfig), false);
83+
ASSERT_EQ(config->Get(TestConfig::kEnumConfig), TestEnum::VALUE1);
84+
ASSERT_EQ(config->Get(TestConfig::kDoubleConfig), 3.14);
7885

7986
// Test setting values
80-
config.Set(TestConfig::kStringConfig, std::string("new_value"));
81-
config.Set(TestConfig::kIntConfig, 100);
82-
config.Set(TestConfig::kBoolConfig, true);
83-
config.Set(TestConfig::kEnumConfig, TestEnum::VALUE2);
84-
config.Set(TestConfig::kDoubleConfig, 2.99);
85-
86-
ASSERT_EQ(config.Get(TestConfig::kStringConfig), "new_value");
87-
ASSERT_EQ(config.Get(TestConfig::kIntConfig), 100);
88-
ASSERT_EQ(config.Get(TestConfig::kBoolConfig), true);
89-
ASSERT_EQ(config.Get(TestConfig::kEnumConfig), TestEnum::VALUE2);
90-
ASSERT_EQ(config.Get(TestConfig::kDoubleConfig), 2.99);
87+
config->Set(TestConfig::kStringConfig, std::string("new_value"));
88+
config->Set(TestConfig::kIntConfig, 100);
89+
config->Set(TestConfig::kBoolConfig, true);
90+
config->Set(TestConfig::kEnumConfig, TestEnum::VALUE2);
91+
config->Set(TestConfig::kDoubleConfig, 2.99);
92+
93+
ASSERT_EQ(config->Get(TestConfig::kStringConfig), "new_value");
94+
ASSERT_EQ(config->Get(TestConfig::kIntConfig), 100);
95+
ASSERT_EQ(config->Get(TestConfig::kBoolConfig), true);
96+
ASSERT_EQ(config->Get(TestConfig::kEnumConfig), TestEnum::VALUE2);
97+
ASSERT_EQ(config->Get(TestConfig::kDoubleConfig), 2.99);
9198

9299
// Test unsetting a value
93-
config.Unset(TestConfig::kIntConfig);
94-
config.Unset(TestConfig::kEnumConfig);
95-
config.Unset(TestConfig::kDoubleConfig);
96-
ASSERT_EQ(config.Get(TestConfig::kIntConfig), 25);
97-
ASSERT_EQ(config.Get(TestConfig::kStringConfig), "new_value");
98-
ASSERT_EQ(config.Get(TestConfig::kBoolConfig), true);
99-
ASSERT_EQ(config.Get(TestConfig::kEnumConfig), TestEnum::VALUE1);
100-
ASSERT_EQ(config.Get(TestConfig::kDoubleConfig), 3.14);
100+
config->Unset(TestConfig::kIntConfig);
101+
config->Unset(TestConfig::kEnumConfig);
102+
config->Unset(TestConfig::kDoubleConfig);
103+
ASSERT_EQ(config->Get(TestConfig::kIntConfig), 25);
104+
ASSERT_EQ(config->Get(TestConfig::kStringConfig), "new_value");
105+
ASSERT_EQ(config->Get(TestConfig::kBoolConfig), true);
106+
ASSERT_EQ(config->Get(TestConfig::kEnumConfig), TestEnum::VALUE1);
107+
ASSERT_EQ(config->Get(TestConfig::kDoubleConfig), 3.14);
101108

102109
// Test resetting all values
103-
config.Reset();
104-
ASSERT_EQ(config.Get(TestConfig::kStringConfig), "default_value");
105-
ASSERT_EQ(config.Get(TestConfig::kIntConfig), 25);
106-
ASSERT_EQ(config.Get(TestConfig::kBoolConfig), false);
107-
ASSERT_EQ(config.Get(TestConfig::kEnumConfig), TestEnum::VALUE1);
108-
ASSERT_EQ(config.Get(TestConfig::kDoubleConfig), 3.14);
110+
config->Reset();
111+
ASSERT_EQ(config->Get(TestConfig::kStringConfig), "default_value");
112+
ASSERT_EQ(config->Get(TestConfig::kIntConfig), 25);
113+
ASSERT_EQ(config->Get(TestConfig::kBoolConfig), false);
114+
ASSERT_EQ(config->Get(TestConfig::kEnumConfig), TestEnum::VALUE1);
115+
ASSERT_EQ(config->Get(TestConfig::kDoubleConfig), 3.14);
109116
}
110117

111118
} // namespace iceberg

0 commit comments

Comments
 (0)