Use enum class for hstore options instead of macros

Author: joto

src/options.cpp

@@ -465,21 +465,21 @@ options_t::options_t(int argc, char *argv[]) : options_t()
             extra_attributes = true;
             break;
         case 'k':
-            if (hstore_mode != HSTORE_NONE) {
+            if (hstore_mode != hstore_column::none) {
                 throw std::runtime_error{"You can not specify both --hstore "
                                          "(-k) and --hstore-all (-j)\n"};
             }
-            hstore_mode = HSTORE_NORM;
+            hstore_mode = hstore_column::norm;
             break;
         case 208:
             hstore_match_only = true;
             break;
         case 'j':
-            if (hstore_mode != HSTORE_NONE) {
+            if (hstore_mode != hstore_column::none) {
                 throw std::runtime_error{"You can not specify both --hstore "
                                          "(-k) and --hstore-all (-j)\n"};
             }
-            hstore_mode = HSTORE_ALL;
+            hstore_mode = hstore_column::all;
             break;
         case 'z':
             hstore_columns.emplace_back(optarg);
@@ -596,15 +596,15 @@ void options_t::check_options()
         throw std::runtime_error{"--drop only makes sense with --slim.\n"};
     }
 
-    if (hstore_mode == HSTORE_NONE && hstore_columns.empty() &&
+    if (hstore_mode == hstore_column::none && hstore_columns.empty() &&
         hstore_match_only) {
         fprintf(stderr,
                 "Warning: --hstore-match-only only makes sense with --hstore, "
                 "--hstore-all, or --hstore-column; ignored.\n");
         hstore_match_only = false;
     }
 
-    if (enable_hstore_index && hstore_mode == HSTORE_NONE &&
+    if (enable_hstore_index && hstore_mode == hstore_column::none &&
         hstore_columns.empty()) {
         fprintf(stderr, "Warning: --hstore-add-index only makes sense with "
                         "hstore enabled.\n");

src/options.hpp

@@ -11,13 +11,16 @@
 #include <string>
 #include <vector>
 
-/* Variants for generation of hstore column */
-/* No hstore column */
-#define HSTORE_NONE 0
-/* create a hstore column for all tags which do not have an exclusive column */
-#define HSTORE_NORM 1
-/* create a hstore column for all tags */
-#define HSTORE_ALL 2
+/// Variants for generation of hstore column
+enum class hstore_column : char
+{
+    /// no hstore column
+    none = 0,
+    /// create hstore column for all tags without exclusive column
+    norm = 1,
+    /// create hstore column for all tags
+    all = 2
+};
 
 /**
  * Database options, not specific to a table
@@ -80,7 +83,7 @@ class options_t
     std::string expire_tiles_filename{"dirty_tiles"};
 
     /// add an additional hstore column with objects key/value pairs, and what type of hstore column
-    int hstore_mode = HSTORE_NONE;
+    hstore_column hstore_mode = hstore_column::none;
 
     bool enable_hstore_index = false; ///< add an index on the hstore column
 

src/output.cpp

@@ -73,7 +73,9 @@ parse_multi_single(pt::ptree const &conf,
     new_opts.tblsmain_index = conf.get("tablespace-index", "");
     new_opts.tblsmain_data = conf.get("tablespace-data", "");
 
-    override_if<int>(new_opts.hstore_mode, "enable-hstore", conf);
+    if (conf.get<bool>("enable-hstore", false)) {
+        new_opts.hstore_mode = hstore_column::norm;
+    }
     override_if<bool>(new_opts.enable_hstore_index, "enable-hstore-index",
                       conf);
     override_if<bool>(new_opts.enable_multi, "enable-multi", conf);

src/table.cpp

@@ -15,15 +15,15 @@
 
 table_t::table_t(std::string const &name, std::string const &type,
                  columns_t const &columns, hstores_t const &hstore_columns,
-                 int const srid, bool const append, int const hstore_mode,
+                 int const srid, bool const append, hstore_column hstore_mode,
                  std::shared_ptr<db_copy_thread_t> const &copy_thread)
 : m_target(std::make_shared<db_target_descr_t>(name.c_str(), "osm_id")),
   m_type(type), m_srid(fmt::to_string(srid)), m_append(append),
   m_hstore_mode(hstore_mode), m_columns(columns),
   m_hstore_columns(hstore_columns), m_copy(copy_thread)
 {
     // if we dont have any columns
-    if (m_columns.empty() && m_hstore_mode != HSTORE_ALL) {
+    if (m_columns.empty() && m_hstore_mode != hstore_column::all) {
         throw std::runtime_error{
             "No columns provided for table {}"_format(name)};
     }
@@ -99,7 +99,7 @@ void table_t::start(std::string const &conninfo, std::string const &table_space)
         }
 
         //add tags column
-        if (m_hstore_mode != HSTORE_NONE) {
+        if (m_hstore_mode != hstore_column::none) {
             sql += "\"tags\" hstore,";
         }
 
@@ -163,7 +163,7 @@ void table_t::generate_copy_column_list()
     }
 
     //add tags column and geom column
-    if (m_hstore_mode != HSTORE_NONE) {
+    if (m_hstore_mode != hstore_column::none) {
         m_target->rows += "tags,way";
         //or just the geom column
     } else {
@@ -264,7 +264,7 @@ void table_t::stop(bool updateable, bool enable_hstore_index,
         if (enable_hstore_index) {
             fmt::print(stderr, "Creating hstore indexes on {}\n",
                        m_target->name);
-            if (m_hstore_mode != HSTORE_NONE) {
+            if (m_hstore_mode != hstore_column::none) {
                 m_sql_conn->exec(
                     "CREATE INDEX ON {} USING GIN (tags) {}"_format(
                         m_target->name, tablespace_clause(table_space_index)));
@@ -303,18 +303,18 @@ void table_t::write_row(osmid_t id, taglist_t const &tags,
     // used to remember which columns have been written out already.
     std::vector<bool> used;
 
-    if (m_hstore_mode != HSTORE_NONE) {
+    if (m_hstore_mode != hstore_column::none) {
         used.assign(tags.size(), false);
     }
 
     //get the regular columns' values
-    write_columns(tags, m_hstore_mode == HSTORE_NORM ? &used : nullptr);
+    write_columns(tags, m_hstore_mode == hstore_column::norm ? &used : nullptr);
 
     //get the hstore columns' values
     write_hstore_columns(tags);
 
     //get the key value pairs for the tags column
-    if (m_hstore_mode != HSTORE_NONE) {
+    if (m_hstore_mode != hstore_column::none) {
         write_tags_column(tags, used);
     }
 

src/table.hpp

@@ -19,7 +19,7 @@ class table_t
 public:
     table_t(std::string const &name, std::string const &type,
             columns_t const &columns, hstores_t const &hstore_columns,
-            int const srid, bool const append, int const hstore_mode,
+            int const srid, bool const append, hstore_column hstore_mode,
             std::shared_ptr<db_copy_thread_t> const &copy_thread);
     table_t(table_t const &other,
             std::shared_ptr<db_copy_thread_t> const &copy_thread);
@@ -81,7 +81,7 @@ class table_t
     std::unique_ptr<pg_conn_t> m_sql_conn;
     std::string m_srid;
     bool m_append;
-    int m_hstore_mode;
+    hstore_column m_hstore_mode;
     columns_t m_columns;
     hstores_t m_hstore_columns;
     std::string m_table_space;

src/tagtransform-c.cpp

@@ -114,7 +114,7 @@ bool c_tagtransform_t::check_key(std::vector<taginfo> const &infos,
     // if we didn't find any tags that we wanted to export
     // and we aren't strictly adhering to the list
     if (!strict) {
-        if (m_options->hstore_mode != HSTORE_NONE) {
+        if (m_options->hstore_mode != hstore_column::none) {
             /* ... but if hstore_match_only is set then don't take this
                  as a reason for keeping the object */
             if (!m_options->hstore_match_only) {

tests/test-output-pgsql-hstore-match-only.cpp

@@ -10,7 +10,7 @@ TEST_CASE("hstore match only import")
     options_t options =
         testing::opt_t().slim().style("hstore-match-only.style");
     options.hstore_match_only = true;
-    options.hstore_mode = HSTORE_NORM;
+    options.hstore_mode = hstore_column::norm;
 
     REQUIRE_NOTHROW(db.run_file(options, "hstore-match-only.osm"));