Merge pull request #1880 from joto/replace-string-id-list

Replace util::string_id_list_t by util::string_joiner_t

Author: lonvia

src/middle-pgsql.cpp

@@ -251,7 +251,7 @@ std::size_t middle_query_pgsql_t::get_way_node_locations_db(
     osmium::WayNodeList *nodes) const
 {
     size_t count = 0;
-    util::string_id_list_t id_list;
+    util::string_joiner_t id_list{',', '\0', '{', '}'};
 
     // get nodes where possible from cache,
     // at the same time build a list for querying missing nodes from DB
@@ -261,7 +261,7 @@ std::size_t middle_query_pgsql_t::get_way_node_locations_db(
             n.set_location(loc);
             ++count;
         } else {
-            id_list.add(n.ref());
+            id_list.add(fmt::to_string(n.ref()));
         }
     }
 
@@ -271,7 +271,7 @@ std::size_t middle_query_pgsql_t::get_way_node_locations_db(
 
     // get any remaining nodes from the DB
     // Nodes must have been written back at this point.
-    auto const res = m_sql_conn.exec_prepared("get_node_list", id_list.get());
+    auto const res = m_sql_conn.exec_prepared("get_node_list", id_list());
     std::unordered_map<osmid_t, osmium::Location> locs;
     for (int i = 0; i < res.num_tuples(); ++i) {
         locs.emplace(
@@ -478,16 +478,16 @@ middle_query_pgsql_t::rel_members_get(osmium::Relation const &rel,
     idlist_t wayidspg;
     if (types & osmium::osm_entity_bits::way) {
         // collect ids from all way members into a list..
-        util::string_id_list_t way_ids;
+        util::string_joiner_t way_ids{',', '\0', '{', '}'};
         for (auto const &member : rel.members()) {
             if (member.type() == osmium::item_type::way) {
-                way_ids.add(member.ref());
+                way_ids.add(fmt::to_string(member.ref()));
             }
         }
 
         // ...and get those ways from database
         if (!way_ids.empty()) {
-            res = m_sql_conn.exec_prepared("get_way_list", way_ids.get());
+            res = m_sql_conn.exec_prepared("get_way_list", way_ids());
             wayidspg = get_ids_from_result(res);
         }
     }

src/util.cpp

@@ -21,18 +21,6 @@
 
 namespace util {
 
-void string_id_list_t::add(osmid_t id)
-{
-    fmt::format_to(std::back_inserter(m_list), "{},", id);
-}
-
-std::string const &string_id_list_t::get()
-{
-    assert(!empty());
-    m_list.back() = '}';
-    return m_list;
-}
-
 std::string human_readable_duration(uint64_t seconds)
 {
     if (seconds < 60) {

src/util.hpp

@@ -65,24 +65,6 @@ class double_to_buffer
     std::array<char, buffer_size> m_buffer{};
 };
 
-/**
- * Class for building a stringified list of ids in the format "{id1,id2,id3}"
- * for use in PostgreSQL queries.
- */
-class string_id_list_t
-{
-public:
-    void add(osmid_t id);
-
-    bool empty() const noexcept { return m_list.size() == 1; }
-
-    std::string const &get();
-
-private:
-    std::string m_list{"{"};
-
-}; // class string_id_list_t
-
 /**
  * Helper class for timing with a granularity of microseconds. The timer will
  * start on construction or it can be started by calling start(). It is stopped
@@ -191,6 +173,14 @@ class string_joiner_t
      */
     void add(std::string const &item);
 
+    bool empty() const noexcept
+    {
+        if (m_before == '\0') {
+            return m_result.empty();
+        }
+        return m_result.size() == 1;
+    }
+
     /// Return result (as value!)
     std::string operator()();
 

tests/test-util.cpp

@@ -46,30 +46,6 @@ TEST_CASE("double_to_buffer 3.141", "[NoDB]")
     REQUIRE(std::strcmp(buffer.c_str(), "3.141") == 0);
 }
 
-TEST_CASE("string_id_list_t with one element", "[NoDB]")
-{
-    util::string_id_list_t list;
-    REQUIRE(list.empty());
-
-    list.add(17);
-
-    REQUIRE_FALSE(list.empty());
-    REQUIRE(list.get() == "{17}");
-}
-
-TEST_CASE("string_id_list_t with several elements", "[NoDB]")
-{
-    util::string_id_list_t list;
-    REQUIRE(list.empty());
-
-    list.add(17);
-    list.add(3);
-    list.add(99);
-
-    REQUIRE_FALSE(list.empty());
-    REQUIRE(list.get() == "{17,3,99}");
-}
-
 TEST_CASE("human readable time durations", "[NoDB]")
 {
     REQUIRE(util::human_readable_duration(0) == "0s");
@@ -116,28 +92,45 @@ TEST_CASE("find_by_name()", "[NoDB]")
 TEST_CASE("Use string_joiner_t with delim only without items", "[NoDB]")
 {
     util::string_joiner_t joiner{','};
+    REQUIRE(joiner.empty());
     REQUIRE(joiner().empty());
 }
 
 TEST_CASE("Use string_joiner_t with all params without items", "[NoDB]")
 {
     util::string_joiner_t joiner{',', '"', '(', ')'};
+    REQUIRE(joiner.empty());
     REQUIRE(joiner().empty());
 }
 
 TEST_CASE("Use string_joiner_t without quote char", "[NoDB]")
 {
     util::string_joiner_t joiner{',', '\0', '(', ')'};
+    REQUIRE(joiner.empty());
     joiner.add("foo");
+    REQUIRE_FALSE(joiner.empty());
     joiner.add("bar");
+    REQUIRE_FALSE(joiner.empty());
     REQUIRE(joiner() == "(foo,bar)");
 }
 
+TEST_CASE("Use string_joiner_t with quote char", "[NoDB]")
+{
+    util::string_joiner_t joiner{',', '-', '(', ')'};
+    REQUIRE(joiner.empty());
+    joiner.add("foo");
+    REQUIRE_FALSE(joiner.empty());
+    joiner.add("bar");
+    REQUIRE_FALSE(joiner.empty());
+    REQUIRE(joiner() == "(-foo-,-bar-)");
+}
+
 TEST_CASE("string_joiner_t without before/after", "[NoDB]")
 {
     util::string_joiner_t joiner{','};
     joiner.add("xxx");
     joiner.add("yyy");
+    REQUIRE_FALSE(joiner.empty());
     REQUIRE(joiner() == "xxx,yyy");
 }