Skip to content

Commit

Permalink
Remove index field from hashmap and properties iterator
Browse files Browse the repository at this point in the history
  • Loading branch information
@pnoltes
pnoltes committed Nov 20, 2023
1 parent caedd79 commit 78d7879
Show file tree
Hide file tree
Showing 8 changed files with 7 additions and 99 deletions.
78 changes: 4 additions & 74 deletions libs/utils/gtest/src/HashMapTestSuite.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -428,7 +428,7 @@ TEST_F(HashMapTestSuite, IterateTest) {
auto* sMap = createStringHashMap(2);
size_t count = 0;
CELIX_STRING_HASH_MAP_ITERATE(sMap, iter) {
EXPECT_EQ(count++, iter.index);
count++;
if (iter.value.longValue == 0) {
EXPECT_STREQ(iter.key, "key0");
} else if (iter.value.longValue == 1) {
Expand All @@ -443,7 +443,7 @@ TEST_F(HashMapTestSuite, IterateTest) {
auto* lMap = createLongHashMap(2);
count = 0;
CELIX_LONG_HASH_MAP_ITERATE(lMap, iter) {
EXPECT_EQ(count++, iter.index);
count++;
if (iter.value.longValue == 0) {
EXPECT_EQ(iter.key, 0);
} else if (iter.value.longValue == 1) {
Expand All @@ -461,15 +461,15 @@ TEST_F(HashMapTestSuite, IterateStressTest) {
auto* sMap = createStringHashMap(testCount);
EXPECT_EQ(testCount, celix_stringHashMap_size(sMap));
int count = 0;
CELIX_STRING_HASH_MAP_ITERATE(sMap, iter) { EXPECT_EQ(iter.index, count++); }
CELIX_STRING_HASH_MAP_ITERATE(sMap, iter) { count++; }
EXPECT_EQ(testCount, count);
testGetEntriesFromStringMap(sMap, 100);
celix_stringHashMap_destroy(sMap);

auto* lMap = createLongHashMap(testCount);
EXPECT_EQ(testCount, celix_longHashMap_size(lMap));
count = 0;
CELIX_LONG_HASH_MAP_ITERATE(lMap, iter) { EXPECT_EQ(iter.index, count++); }
CELIX_LONG_HASH_MAP_ITERATE(lMap, iter) { count++; }
EXPECT_EQ(testCount, count);
testGetEntriesFromLongMap(lMap, 100);
celix_longHashMap_destroy(lMap);
Expand Down Expand Up @@ -506,72 +506,6 @@ TEST_F(HashMapTestSuite, IterateStressCapacityAndLoadFactorTest) {
celix_longHashMap_destroy(lMap);
}

TEST_F(HashMapTestSuite, IterateWithRemoveAtIndex0Test) {
auto* sMap = createStringHashMap(6);
auto iter1 = celix_stringHashMap_begin(sMap);
while (!celix_stringHashMapIterator_isEnd(&iter1)) {
if (iter1.index == 0) {
// note only removing entries where the iter key is even
celix_stringHashMapIterator_remove(&iter1);
} else {
FAIL() << "Should not be reached, because index 0 is always removed";
}
}
EXPECT_EQ(0, celix_stringHashMap_size(sMap));
EXPECT_TRUE(celix_stringHashMapIterator_isEnd(&iter1));
celix_stringHashMapIterator_next(&iter1);
EXPECT_TRUE(celix_stringHashMapIterator_isEnd(&iter1));
celix_stringHashMap_destroy(sMap);

auto* lMap = createLongHashMap(6);
auto iter2 = celix_longHashMap_begin(lMap);
while (!celix_longHashMapIterator_isEnd(&iter2)) {
if (iter2.index % 2 == 0) {
// note only removing entries where the iter index is even
celix_longHashMapIterator_remove(&iter2);
} else {
FAIL() << "Should not be reached, because index 0 is always removed";
}
}
EXPECT_EQ(0, celix_longHashMap_size(lMap));
EXPECT_TRUE(celix_longHashMapIterator_isEnd(&iter2));
celix_longHashMapIterator_next(&iter2); // calling next on end iter, does nothing
EXPECT_TRUE(celix_longHashMapIterator_isEnd(&iter2));
celix_longHashMap_destroy(lMap);
}

TEST_F(HashMapTestSuite, IterateWithRemoveAtIndex4Test) {
celix_autoptr(celix_string_hash_map_t) sMap = createStringHashMap(6);
auto iter1 = celix_stringHashMap_begin(sMap);
while (!celix_stringHashMapIterator_isEnd(&iter1)) {
if (iter1.index == 4) {
// note only removing entries where the iter key is even
celix_stringHashMapIterator_remove(&iter1);
} else {
celix_stringHashMapIterator_next(&iter1);
}
}
EXPECT_EQ(4, celix_stringHashMap_size(sMap));
EXPECT_TRUE(celix_stringHashMapIterator_isEnd(&iter1));
celix_stringHashMapIterator_next(&iter1);
EXPECT_TRUE(celix_stringHashMapIterator_isEnd(&iter1));

celix_autoptr(celix_long_hash_map_t) lMap = createLongHashMap(6);
auto iter2 = celix_longHashMap_begin(lMap);
while (!celix_longHashMapIterator_isEnd(&iter2)) {
if (iter2.index == 4) {
// note only removing entries where the iter index is even
celix_longHashMapIterator_remove(&iter2);
} else {
celix_longHashMapIterator_next(&iter2);
}
}
EXPECT_EQ(4, celix_longHashMap_size(lMap));
EXPECT_TRUE(celix_longHashMapIterator_isEnd(&iter2));
celix_longHashMapIterator_next(&iter2); // calling next on end iter, does nothing
EXPECT_TRUE(celix_longHashMapIterator_isEnd(&iter2));
}

TEST_F(HashMapTestSuite, IterateEndTest) {
auto* sMap1 = createStringHashMap(0);
auto* sMap2 = createStringHashMap(6);
Expand All @@ -583,10 +517,6 @@ TEST_F(HashMapTestSuite, IterateEndTest) {
auto lIter1 = celix_longHashMap_end(lMap1);
auto lIter2 = celix_longHashMap_end(lMap2);

EXPECT_EQ(sIter1.index, 0);
EXPECT_EQ(sIter2.index, 6);
EXPECT_EQ(lIter1.index, 0);
EXPECT_EQ(lIter2.index, 6);
EXPECT_TRUE(celix_stringHashMapIterator_isEnd(&sIter1));
EXPECT_TRUE(celix_stringHashMapIterator_isEnd(&sIter2));
EXPECT_TRUE(celix_longHashMapIterator_isEnd(&lIter1));
Expand Down
8 changes: 2 additions & 6 deletions libs/utils/gtest/src/PropertiesTestSuite.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -550,21 +550,17 @@ TEST_F(PropertiesTestSuite, EndOfPropertiesTest) {
auto* props = celix_properties_create();
celix_properties_set(props, "key1", "value1");
celix_properties_set(props, "key2", "value2");

celix_properties_iterator_t endIter = celix_properties_end(props);
EXPECT_EQ(endIter.index, 2);
EXPECT_TRUE(celix_propertiesIterator_isEnd(&endIter));

celix_properties_destroy(props);
}

TEST_F(PropertiesTestSuite, EndOfEmptyPropertiesTest) {
auto* props = celix_properties_create();

celix_properties_iterator_t endIter = celix_properties_end(props);
EXPECT_EQ(endIter.index, 0);
EXPECT_TRUE(celix_propertiesIterator_isEnd(&endIter));

celix_properties_iterator_t beginIter = celix_properties_begin(props);
EXPECT_TRUE(celix_propertiesIterator_isEnd(&beginIter)); //empty properties: begin == end
celix_properties_destroy(props);
}

Expand Down
2 changes: 1 addition & 1 deletion libs/utils/include/celix/Properties.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ namespace celix {
}
}

celix_properties_iterator_t iter{.index = 0, .key = nullptr, .entry = {}, ._data = {}};
celix_properties_iterator_t iter{.key = nullptr, .entry = {}, ._data = {}};
};


Expand Down
2 changes: 0 additions & 2 deletions libs/utils/include/celix_long_hash_map.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -53,8 +53,6 @@ typedef struct celix_long_hash_map celix_long_hash_map_t;
* @Brief long hash map iterator, which contains a hash map entry's keu and value.
*/
typedef struct celix_long_hash_map_iterator {
size_t index; /**< The index of the iterator. Starts at 0, ends at map size (so beyond the last element). When an
entry is removed, the index value is not changed. */
long key; /**< The key of the hash map entry. */
celix_hash_map_value_t value; /**< The value of the hash map entry. */

Expand Down
5 changes: 0 additions & 5 deletions libs/utils/include/celix_properties.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -93,11 +93,6 @@ typedef struct celix_properties_entry {
* @brief Represents an iterator for iterating over the entries in a celix_properties_t object.
*/
typedef struct celix_properties_iterator {
/**
* @brief The index of the current iterator.
*/
size_t index;

/**
* @brief The current key.
*/
Expand Down
2 changes: 0 additions & 2 deletions libs/utils/include/celix_string_hash_map.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -52,8 +52,6 @@ typedef struct celix_string_hash_map celix_string_hash_map_t;
* @Brief string hash map iterator, which contains a hash map entry's keu and value.
*/
typedef struct celix_string_hash_map_iterator {
size_t index; /**< The index of the iterator. Starts at 0, ends at map size (so beyond the last element). When an
entry is removed, the index value is not changed. */
const char* key; /**< The key of the hash map entry. */
celix_hash_map_value_t value; /**< The value of the hash map entry. */

Expand Down
6 changes: 0 additions & 6 deletions libs/utils/src/celix_hash_map.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -688,7 +688,6 @@ celix_string_hash_map_iterator_t celix_stringHashMap_end(const celix_string_hash
celix_string_hash_map_iterator_t iter;
iter._internal[0] = (void*)&map->genericMap;
iter._internal[1] = NULL;
iter.index = map->genericMap.size;
iter.key = "";
memset(&iter.value, 0, sizeof(iter.value));
return iter;
Expand All @@ -698,7 +697,6 @@ celix_long_hash_map_iterator_t celix_longHashMap_end(const celix_long_hash_map_t
celix_long_hash_map_iterator_t iter;
iter._internal[0] = (void*)&map->genericMap;
iter._internal[1] = NULL;
iter.index = map->genericMap.size;
iter.key = 0L;
memset(&iter.value, 0, sizeof(iter.value));
return iter;
Expand All @@ -715,7 +713,6 @@ bool celix_longHashMapIterator_isEnd(const celix_long_hash_map_iterator_t* iter)
void celix_stringHashMapIterator_next(celix_string_hash_map_iterator_t* iter) {
const celix_hash_map_t* map = iter->_internal[0];
celix_hash_map_entry_t *entry = iter->_internal[1];
iter->index = iter->index == map->size ? map->size : iter->index + 1; //increment but not beyond size
entry = celix_hashMap_nextEntry(map, entry);
if (entry) {
iter->_internal[1] = entry;
Expand All @@ -731,7 +728,6 @@ void celix_stringHashMapIterator_next(celix_string_hash_map_iterator_t* iter) {
void celix_longHashMapIterator_next(celix_long_hash_map_iterator_t* iter) {
const celix_hash_map_t* map = iter->_internal[0];
celix_hash_map_entry_t *entry = iter->_internal[1];
iter->index = iter->index == map->size ? map->size : iter->index + 1; //increment but not beyond size
entry = celix_hashMap_nextEntry(map, entry);
if (entry) {
iter->_internal[1] = entry;
Expand Down Expand Up @@ -764,7 +760,6 @@ void celix_stringHashMapIterator_remove(celix_string_hash_map_iterator_t* iter)
const char* key = entry->key.strKey;
celix_stringHashMapIterator_next(iter);
celix_hashMap_remove(map, key, 0);
iter->index -= 1; //decrement index, because of remove
}

void celix_longHashMapIterator_remove(celix_long_hash_map_iterator_t* iter) {
Expand All @@ -773,7 +768,6 @@ void celix_longHashMapIterator_remove(celix_long_hash_map_iterator_t* iter) {
long key = entry->key.longKey;
celix_longHashMapIterator_next(iter);
celix_hashMap_remove(map, NULL, key);
iter->index -= 1; //decrement index, because of remove
}

static int celix_hashMap_nrOfEntriesInBucket(const celix_hash_map_t* map, int bucketIndex) {
Expand Down
3 changes: 0 additions & 3 deletions libs/utils/src/properties.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -887,7 +887,6 @@ celix_properties_iterator_t celix_properties_begin(const celix_properties_t* pro
internalIter.mapIter = celix_stringHashMap_begin(properties->map);
internalIter.props = properties;

iter.index = 0;
if (celix_stringHashMapIterator_isEnd(&internalIter.mapIter)) {
iter.key = NULL;
memset(&iter.entry, 0, sizeof(iter.entry));
Expand All @@ -908,7 +907,6 @@ celix_properties_iterator_t celix_properties_end(const celix_properties_t* prope

celix_properties_iterator_t iter;
memset(&iter, 0, sizeof(iter));
iter.index = internalIter.mapIter.index;
memcpy(iter._data, &internalIter, sizeof(internalIter));
return iter;
}
Expand All @@ -918,7 +916,6 @@ void celix_propertiesIterator_next(celix_properties_iterator_t* iter) {
memcpy(&internalIter, iter->_data, sizeof(internalIter));
celix_stringHashMapIterator_next(&internalIter.mapIter);
memcpy(iter->_data, &internalIter, sizeof(internalIter));
iter->index = internalIter.mapIter.index;
if (celix_stringHashMapIterator_isEnd(&internalIter.mapIter)) {
iter->key = NULL;
memset(&iter->entry, 0, sizeof(iter->entry));
Expand Down

0 comments on commit 78d7879

Please sign in to comment.