Skip to content

Commit

Permalink
gh-674: Remove defaultValue arg from properties get* functions with a…
Browse files Browse the repository at this point in the history
… ptr return
  • Loading branch information
pnoltes committed Apr 1, 2024
1 parent 3989ce1 commit 10473c5
Show file tree
Hide file tree
Showing 4 changed files with 27 additions and 35 deletions.
31 changes: 14 additions & 17 deletions libs/utils/gtest/src/PropertiesTestSuite.cc
Original file line number Diff line number Diff line change
Expand Up @@ -332,7 +332,7 @@ TEST_F(PropertiesTestSuite, GetSetOverwrite) {
EXPECT_EQ(CELIX_SUCCESS, celix_properties_setBool(props, "key", false));
EXPECT_EQ(false, celix_properties_getAsBool(props, "key", true));
EXPECT_EQ(CELIX_SUCCESS, celix_properties_assignVersion(props, "key", version));
EXPECT_EQ(version, celix_properties_getVersion(props, "key", nullptr));
EXPECT_EQ(version, celix_properties_getVersion(props, "key"));
celix_properties_set(props, "key", "last");

celix_properties_destroy(props);
Expand Down Expand Up @@ -494,25 +494,23 @@ TEST_F(PropertiesTestSuite, GetVersionTest) {
// Test getting a version property
auto* expected = celix_version_create(1, 2, 3, "test");
celix_properties_setVersion(properties, "key", expected);
const auto* actual = celix_properties_getVersion(properties, "key", nullptr);
const auto* actual = celix_properties_getVersion(properties, "key");
EXPECT_EQ(celix_version_getMajor(expected), celix_version_getMajor(actual));
EXPECT_EQ(celix_version_getMinor(expected), celix_version_getMinor(actual));
EXPECT_EQ(celix_version_getMicro(expected), celix_version_getMicro(actual));
EXPECT_STREQ(celix_version_getQualifier(expected), celix_version_getQualifier(actual));

// Test getting a non-version property
celix_properties_set(properties, "key2", "value");
actual = celix_properties_getVersion(properties, "key2", emptyVersion);
EXPECT_EQ(celix_version_getMajor(actual), 0);
EXPECT_EQ(celix_version_getMinor(actual), 0);
EXPECT_EQ(celix_version_getMicro(actual), 0);
EXPECT_STREQ(celix_version_getQualifier(actual), "");
EXPECT_EQ(celix_properties_getVersion(properties, "non-existent", nullptr), nullptr);
actual = celix_properties_getVersion(properties, "key2");
EXPECT_EQ(nullptr, actual);
actual = celix_properties_getVersion(properties, "non-existent");
EXPECT_EQ(nullptr, actual);
celix_version_destroy(expected);

// Test setting without copy
celix_properties_assignVersion(properties, "key3", celix_version_create(3, 3, 3, ""));
actual = celix_properties_getVersion(properties, "key3", emptyVersion);
actual = celix_properties_getVersion(properties, "key3");
EXPECT_EQ(celix_version_getMajor(actual), 3);
EXPECT_EQ(celix_version_getMinor(actual), 3);
EXPECT_EQ(celix_version_getMicro(actual), 3);
Expand Down Expand Up @@ -632,7 +630,7 @@ TEST_F(PropertiesTestSuite, SetEntryWithLargeStringValueTest) {
celix_version_create(1, 2, 3, "a-qualifier-that-is-longer-than-20-characters");
celix_properties_setVersion(props1, "key2", version);
EXPECT_EQ(CELIX_PROPERTIES_VALUE_TYPE_VERSION, celix_properties_getType(props1, "key2"));
EXPECT_EQ(0, celix_version_compareTo(version, celix_properties_getVersion(props1, "key2", nullptr)));
EXPECT_EQ(0, celix_version_compareTo(version, celix_properties_getVersion(props1, "key2")));
}


Expand Down Expand Up @@ -748,29 +746,28 @@ TEST_F(PropertiesTestSuite, GetLongDoubleBoolVersionAndStringTest) {
celix_properties_assignVersion(props, "version", version);

// check if the values are correctly returned
EXPECT_STREQ("value", celix_properties_getString(props, "str", nullptr));
EXPECT_STREQ("value", celix_properties_getString(props, "str"));
EXPECT_EQ(42, celix_properties_getLong(props, "long", -1L));
EXPECT_DOUBLE_EQ(3.14, celix_properties_getDouble(props, "double", -1.0));
EXPECT_EQ(true, celix_properties_getBool(props, "bool", false));
EXPECT_EQ(version, celix_properties_getVersion(props, "version", nullptr));
EXPECT_EQ(version, celix_properties_getVersion(props, "version"));

// check if the values are correctly returned if value is not found
EXPECT_EQ(nullptr, celix_properties_getString(props, "non-existing", nullptr));
EXPECT_EQ(nullptr, celix_properties_getString(props, "non-existing"));
EXPECT_EQ(-1L, celix_properties_getLong(props, "non-existing", -1L));
EXPECT_DOUBLE_EQ(-1.0, celix_properties_getDouble(props, "non-existing", -1.0));
EXPECT_EQ(false, celix_properties_getBool(props, "non-existing", false));
EXPECT_EQ(nullptr, celix_properties_getVersion(props, "non-existing", nullptr));
EXPECT_EQ(nullptr, celix_properties_getVersion(props, "non-existing"));

// check if the values are correctly returned if the found value is not of the correct type
EXPECT_EQ(nullptr, celix_properties_getString(props, "long", nullptr));
EXPECT_EQ(nullptr, celix_properties_getString(props, "long"));
EXPECT_EQ(-1L, celix_properties_getLong(props, "str", -1L));
EXPECT_DOUBLE_EQ(-1.0, celix_properties_getDouble(props, "str", -1.0));
EXPECT_EQ(false, celix_properties_getBool(props, "str", false));
EXPECT_EQ(nullptr, celix_properties_getVersion(props, "str", nullptr));
EXPECT_EQ(nullptr, celix_properties_getVersion(props, "str"));

// check if a default ptr is correctly returned if value is not found for string and version
EXPECT_EQ("default", celix_properties_get(props, "non-existing", "default"));
EXPECT_EQ(version, celix_properties_getVersion(props, "non-existing", version));
}

TEST_F(PropertiesTestSuite, LongArrayListTest) {
Expand Down
4 changes: 2 additions & 2 deletions libs/utils/include/celix/Properties.h
Original file line number Diff line number Diff line change
Expand Up @@ -321,7 +321,7 @@ namespace celix {
* requested type.
*/
std::string getString(const std::string& key, const std::string& defaultValue = {}) const {
const char* found = celix_properties_getString(cProps.get(), key.c_str(), nullptr);
const char* found = celix_properties_getString(cProps.get(), key.c_str());
return found == nullptr ? std::string{defaultValue} : std::string{found};
}

Expand Down Expand Up @@ -435,7 +435,7 @@ namespace celix {
* freed.
*/
celix::Version getVersion(const std::string& key, celix::Version defaultValue = {}) const {
auto* v = celix_properties_getVersion(cProps.get(), key.c_str(), nullptr);
auto* v = celix_properties_getVersion(cProps.get(), key.c_str());
if (v) {
return celix::Version{celix_version_getMajor(v),
celix_version_getMinor(v),
Expand Down
13 changes: 5 additions & 8 deletions libs/utils/include/celix_properties.h
Original file line number Diff line number Diff line change
Expand Up @@ -264,12 +264,10 @@ CELIX_UTILS_EXPORT celix_status_t celix_properties_assign(celix_properties_t* pr
* @brief Get the value of a property, if the property is set and the underlying type is a string.
* @param[in] properties The property set to search.
* @param[in] key The key of the property to get.
* @param[in] defaultValue The value to return if the property is not set or the value is not a string.
* @return The value of the property, or the default value if the property is not set or the value is not of the
* requested type.
* @return The value of the property, or NULL if the property is not set or the value is not of the requested type.
*/
CELIX_UTILS_EXPORT const char*
celix_properties_getString(const celix_properties_t* properties, const char* key, const char* defaultValue);
celix_properties_getString(const celix_properties_t* properties, const char* key);

/**
* @brief Get the string value or string representation of a property.
Expand Down Expand Up @@ -493,12 +491,11 @@ CELIX_UTILS_EXPORT celix_status_t celix_properties_assignVersion(celix_propertie
*
* @param[in] properties The property set to search.
* @param[in] key The key of the property to get.
* @param[in] defaultValue The value to return if the property is not set or if the value is not a Celix version.
* @return A const pointer to the Celix version if it is present and valid, or the provided default value if the
* @return A const pointer to the Celix version if it is present and valid, or NULL if the
* property is not set or the value is not a valid Celix version. The returned pointer should not be modified or freed.
*/
CELIX_UTILS_EXPORT const celix_version_t*
celix_properties_getVersion(const celix_properties_t* properties, const char* key, const celix_version_t* defaultValue);
CELIX_UTILS_EXPORT const celix_version_t* celix_properties_getVersion(const celix_properties_t* properties,
const char* key);

/**
* @brief Get a value of a property as a copied Celix version.
Expand Down
14 changes: 6 additions & 8 deletions libs/utils/src/properties.c
Original file line number Diff line number Diff line change
Expand Up @@ -741,13 +741,12 @@ void celix_properties_unset(celix_properties_t* properties, const char* key) {
}

const char* celix_properties_getString(const celix_properties_t* properties,
const char* key,
const char* defaultValue) {
const char* key) {
const celix_properties_entry_t* entry = celix_properties_getEntry(properties, key);
if (entry != NULL && entry->valueType == CELIX_PROPERTIES_VALUE_TYPE_STRING) {
if (entry && entry->valueType == CELIX_PROPERTIES_VALUE_TYPE_STRING) {
return entry->typed.strValue;
}
return defaultValue;
return NULL;
}

const char* celix_properties_getAsString(const celix_properties_t* properties,
Expand Down Expand Up @@ -864,13 +863,12 @@ celix_status_t celix_properties_setBool(celix_properties_t* props, const char* k
}

const celix_version_t* celix_properties_getVersion(const celix_properties_t* properties,
const char* key,
const celix_version_t* defaultValue) {
const char* key) {
const celix_properties_entry_t* entry = celix_properties_getEntry(properties, key);
if (entry != NULL && entry->valueType == CELIX_PROPERTIES_VALUE_TYPE_VERSION) {
if (entry && entry->valueType == CELIX_PROPERTIES_VALUE_TYPE_VERSION) {
return entry->typed.versionValue;
}
return defaultValue;
return NULL;
}

celix_status_t celix_properties_getAsVersion(const celix_properties_t* properties,
Expand Down

0 comments on commit 10473c5

Please sign in to comment.