From a67f8c14721d1da7a0feb9cb0aa3ba862c98df6d Mon Sep 17 00:00:00 2001 From: rr- Date: Wed, 24 Feb 2016 21:24:23 +0100 Subject: [PATCH] Change Tags to preserve order of insertion --- src/tags.cc | 55 +++++++++++++++++++ src/tags.h | 21 ++++++- src/tags_handlers/insertion_tags_handler.cc | 4 +- .../modification_tags_handler.cc | 2 +- src/tags_handlers/removal_tags_handler.cc | 4 +- .../insertion_tags_handler_test.cc | 4 +- .../modification_tags_handler_test.cc | 14 +++-- .../removal_tags_handler_test.cc | 11 ++-- tests/tags_test.cc | 42 ++++++++++++++ 9 files changed, 139 insertions(+), 18 deletions(-) create mode 100644 src/tags.cc create mode 100644 tests/tags_test.cc diff --git a/src/tags.cc b/src/tags.cc new file mode 100644 index 0000000..78a697e --- /dev/null +++ b/src/tags.cc @@ -0,0 +1,55 @@ +#include "tags.h" +#include + +using namespace opustags; + +Tags::Tags() : max_index(0) +{ +} + +const std::vector> Tags::get_all() const +{ + std::vector keys; + for (const auto &kv : key_to_value) + keys.push_back(kv.first); + + std::sort( + keys.begin(), + keys.end(), + [&](const std::string &a, const std::string &b) + { + return key_to_index.at(a) < key_to_index.at(b); + }); + + std::vector> result; + for (const auto &key : keys) { + result.push_back( + std::make_tuple( + key, key_to_value.at(key))); + } + + return result; +} + +std::string Tags::get(const std::string &key) const +{ + return key_to_value.at(key); +} + +void Tags::set(const std::string &key, const std::string &value) +{ + key_to_value[key] = value; + key_to_index[key] = max_index; + max_index++; +} + +void Tags::remove(const std::string &key) +{ + key_to_value.erase(key); + key_to_index.erase(key); +} + +bool Tags::contains(const std::string &key) const +{ + return key_to_value.find(key) != key_to_value.end(); +} diff --git a/src/tags.h b/src/tags.h index 9e34bed..8df6ff0 100644 --- a/src/tags.h +++ b/src/tags.h @@ -1,9 +1,28 @@ #pragma once #include +#include +#include namespace opustags { - using Tags = std::map; + // A std::map adapter that keeps the order of insertion. + class Tags final + { + public: + Tags(); + + const std::vector> get_all() const; + + std::string get(const std::string &key) const; + void set(const std::string &key, const std::string &value); + void remove(const std::string &key); + bool contains(const std::string &key) const; + + private: + std::map key_to_value; + std::map key_to_index; + size_t max_index; + }; } diff --git a/src/tags_handlers/insertion_tags_handler.cc b/src/tags_handlers/insertion_tags_handler.cc index 81c9f5e..51bdab9 100644 --- a/src/tags_handlers/insertion_tags_handler.cc +++ b/src/tags_handlers/insertion_tags_handler.cc @@ -13,9 +13,9 @@ InsertionTagsHandler::InsertionTagsHandler( bool InsertionTagsHandler::edit_impl(Tags &tags) { - if (tags.find(tag_key) != tags.end()) + if (tags.contains(tag_key)) throw TagAlreadyExistsError(tag_key); - tags[tag_key] = tag_value; + tags.set(tag_key, tag_value); return true; } diff --git a/src/tags_handlers/modification_tags_handler.cc b/src/tags_handlers/modification_tags_handler.cc index 2ea8320..6f38065 100644 --- a/src/tags_handlers/modification_tags_handler.cc +++ b/src/tags_handlers/modification_tags_handler.cc @@ -12,6 +12,6 @@ ModificationTagsHandler::ModificationTagsHandler( bool ModificationTagsHandler::edit_impl(Tags &tags) { - tags[tag_key] = tag_value; + tags.set(tag_key, tag_value); return true; } diff --git a/src/tags_handlers/removal_tags_handler.cc b/src/tags_handlers/removal_tags_handler.cc index f3d31ff..730a9e1 100644 --- a/src/tags_handlers/removal_tags_handler.cc +++ b/src/tags_handlers/removal_tags_handler.cc @@ -11,9 +11,9 @@ RemovalTagsHandler::RemovalTagsHandler( bool RemovalTagsHandler::edit_impl(Tags &tags) { - if (tags.find(tag_key) == tags.end()) + if (!tags.contains(tag_key)) throw TagDoesNotExistError(tag_key); - tags.erase(tag_key); + tags.remove(tag_key); return true; } diff --git a/tests/tags_handlers/insertion_tags_handler_test.cc b/tests/tags_handlers/insertion_tags_handler_test.cc index 9167932..775239d 100644 --- a/tests/tags_handlers/insertion_tags_handler_test.cc +++ b/tests/tags_handlers/insertion_tags_handler_test.cc @@ -13,7 +13,7 @@ TEST_CASE("Insertion tags handler test") streamno, expected_tag_key, expected_tag_value); REQUIRE(handler.edit(streamno, tags)); - REQUIRE(tags.size() == 1); - REQUIRE(tags[expected_tag_key] == expected_tag_value); + REQUIRE(tags.get_all().size() == 1); + REQUIRE(tags.get(expected_tag_key) == expected_tag_value); REQUIRE_THROWS(handler.edit(streamno, tags)); } diff --git a/tests/tags_handlers/modification_tags_handler_test.cc b/tests/tags_handlers/modification_tags_handler_test.cc index 4f84eca..9665796 100644 --- a/tests/tags_handlers/modification_tags_handler_test.cc +++ b/tests/tags_handlers/modification_tags_handler_test.cc @@ -11,18 +11,20 @@ TEST_CASE("Modification tags handler test") const auto dummy_value = "dummy"; const auto new_value = "dummy 2"; - Tags tags = {{first_tag_key, dummy_value}}; - REQUIRE(tags.size() == 1); + Tags tags; + tags.set(first_tag_key, dummy_value); + + REQUIRE(tags.get_all().size() == 1); // setting nonexistent keys adds them ModificationTagsHandler handler1(streamno, other_tag_key, dummy_value); REQUIRE(handler1.edit(streamno, tags)); - REQUIRE(tags.size() == 2); - REQUIRE(tags[other_tag_key] == dummy_value); + REQUIRE(tags.get_all().size() == 2); + REQUIRE(tags.get(other_tag_key) == dummy_value); // setting existing keys overrides their values ModificationTagsHandler handler2(streamno, other_tag_key, new_value); REQUIRE(handler2.edit(streamno, tags)); - REQUIRE(tags.size() == 2); - REQUIRE(tags[other_tag_key] == new_value); + REQUIRE(tags.get_all().size() == 2); + REQUIRE(tags.get(other_tag_key) == new_value); } diff --git a/tests/tags_handlers/removal_tags_handler_test.cc b/tests/tags_handlers/removal_tags_handler_test.cc index bc1b54e..00b3d1c 100644 --- a/tests/tags_handlers/removal_tags_handler_test.cc +++ b/tests/tags_handlers/removal_tags_handler_test.cc @@ -11,10 +11,13 @@ TEST_CASE("Removal tags handler test") const auto dummy_value = "dummy"; RemovalTagsHandler handler(streamno, expected_tag_key); - Tags tags = {{expected_tag_key, dummy_value}, {other_tag_key, dummy_value}}; - REQUIRE(tags.size() == 2); + Tags tags; + tags.set(expected_tag_key, dummy_value); + tags.set(other_tag_key, dummy_value); + + REQUIRE(tags.get_all().size() == 2); REQUIRE(handler.edit(streamno, tags)); - REQUIRE(tags.size() == 1); - REQUIRE(tags.find(other_tag_key) != tags.end()); + REQUIRE(tags.get_all().size() == 1); + REQUIRE(tags.contains(other_tag_key)); REQUIRE_THROWS(handler.edit(streamno, tags)); } diff --git a/tests/tags_test.cc b/tests/tags_test.cc new file mode 100644 index 0000000..ea8f50e --- /dev/null +++ b/tests/tags_test.cc @@ -0,0 +1,42 @@ +#include "tags.h" +#include "catch.h" + +using namespace opustags; + +TEST_CASE("Tag manipulation test") +{ + SECTION("Basic operations") { + Tags tags; + REQUIRE(!tags.contains("a")); + tags.set("a", "1"); + REQUIRE(tags.get("a") == "1"); + REQUIRE(tags.contains("a")); + tags.remove("a"); + REQUIRE(!tags.contains("a")); + REQUIRE_THROWS(tags.get("a")); + } + + SECTION("Maintaing order of insertions") { + Tags tags; + tags.set("z", "1"); + tags.set("y", "2"); + tags.set("x", "3"); + tags.set("y", "4"); + + REQUIRE(tags.get_all().size() == 3); + REQUIRE(std::get<0>(tags.get_all()[0]) == "z"); + REQUIRE(std::get<0>(tags.get_all()[1]) == "x"); + REQUIRE(std::get<0>(tags.get_all()[2]) == "y"); + + tags.remove("z"); + REQUIRE(tags.get_all().size() == 2); + REQUIRE(std::get<0>(tags.get_all()[0]) == "x"); + REQUIRE(std::get<0>(tags.get_all()[1]) == "y"); + + tags.set("gamma", "5"); + REQUIRE(tags.get_all().size() == 3); + REQUIRE(std::get<0>(tags.get_all()[0]) == "x"); + REQUIRE(std::get<0>(tags.get_all()[1]) == "y"); + REQUIRE(std::get<0>(tags.get_all()[2]) == "gamma"); + } +}