From f46803a8b669829fa8115bb5720d4aa38325cb04 Mon Sep 17 00:00:00 2001
From: Alexey Rybalchenko <alexryba@gmail.com>
Date: Mon, 13 May 2019 16:48:24 +0200
Subject: [PATCH] Remove unused methods from FairMQProgOptions

---
 fairmq/FairMQChannel.cxx               |   7 ++
 fairmq/options/FairMQProgOptions.cxx   | 106 ++++++++-----------------
 fairmq/options/FairMQProgOptions.h     |  56 +++----------
 fairmq/options/FairProgOptionsHelper.h |  10 +--
 fairmq/options/runConfigEx.cxx         |   6 --
 5 files changed, 55 insertions(+), 130 deletions(-)

diff --git a/fairmq/FairMQChannel.cxx b/fairmq/FairMQChannel.cxx
index 44ab1918..fd3fa301 100644
--- a/fairmq/FairMQChannel.cxx
+++ b/fairmq/FairMQChannel.cxx
@@ -432,6 +432,13 @@ try {
         return true;
     }
 
+    // validate channel name
+    if (fName.find(".") != string::npos) {
+        ss << "INVALID";
+        LOG(error) << "channel name must not contain '.'";
+        return false;
+    }
+
     // validate socket type
     const set<string> socketTypes{ "sub", "pub", "pull", "push", "req", "rep", "xsub", "xpub", "dealer", "router", "pair" };
     if (socketTypes.find(fType) == socketTypes.end()) {
diff --git a/fairmq/options/FairMQProgOptions.cxx b/fairmq/options/FairMQProgOptions.cxx
index 8b2a2b5e..23965316 100644
--- a/fairmq/options/FairMQProgOptions.cxx
+++ b/fairmq/options/FairMQProgOptions.cxx
@@ -40,7 +40,7 @@ FairMQProgOptions::FairMQProgOptions()
     , fGeneralOptions("General options")
     , fMQOptions("FairMQ device options")
     , fParserOptions("FairMQ channel config parser options")
-    , fConfigMutex()
+    , fMtx()
     , fChannelInfo()
     , fChannelKeyMap()
     , fUnregisteredOptions()
@@ -89,8 +89,7 @@ int FairMQProgOptions::ParseAll(const vector<string>& cmdLineArgs, bool allowUnr
 {
     vector<const char*> argv(cmdLineArgs.size());
 
-    transform(cmdLineArgs.begin(), cmdLineArgs.end(), argv.begin(), [](const string& str)
-    {
+    transform(cmdLineArgs.begin(), cmdLineArgs.end(), argv.begin(), [](const string& str) {
         return str.c_str();
     });
 
@@ -102,20 +101,17 @@ int FairMQProgOptions::ParseAll(const int argc, char const* const* argv, bool al
     ParseCmdLine(argc, argv, allowUnregistered);
 
     // if this option is provided, handle them and return stop value
-    if (fVarMap.count("help"))
-    {
+    if (fVarMap.count("help")) {
         cout << fAllOptions << endl;
         return 1;
     }
     // if this option is provided, handle them and return stop value
-    if (fVarMap.count("print-options"))
-    {
+    if (fVarMap.count("print-options")) {
         PrintOptionsRaw();
         return 1;
     }
     // if these options are provided, do no further checks and let the device handle them
-    if (fVarMap.count("print-channels") || fVarMap.count("version"))
-    {
+    if (fVarMap.count("print-channels") || fVarMap.count("version")) {
         fair::Logger::SetConsoleSeverity("nolog");
         return 0;
     }
@@ -127,13 +123,10 @@ int FairMQProgOptions::ParseAll(const int argc, char const* const* argv, bool al
     string verbosity = GetValue<string>("verbosity");
     fair::Logger::SetVerbosity(verbosity);
 
-    if (logFile != "")
-    {
+    if (logFile != "") {
         fair::Logger::InitFileSink(severity, logFile);
         fair::Logger::SetConsoleSeverity("nolog");
-    }
-    else
-    {
+    } else {
         fair::Logger::SetConsoleColor(color);
         fair::Logger::SetConsoleSeverity(severity);
     }
@@ -141,39 +134,27 @@ int FairMQProgOptions::ParseAll(const int argc, char const* const* argv, bool al
     string idForParser;
 
     // check if config-key for config parser is provided
-    if (fVarMap.count("config-key"))
-    {
+    if (fVarMap.count("config-key")) {
         idForParser = fVarMap["config-key"].as<string>();
-    }
-    else if (fVarMap.count("id"))
-    {
+    } else if (fVarMap.count("id")) {
         idForParser = fVarMap["id"].as<string>();
     }
 
     // check if any config parser is selected
-    try
-    {
-        if (fVarMap.count("mq-config"))
-        {
+    try {
+        if (fVarMap.count("mq-config")) {
             LOG(debug) << "mq-config: Using default JSON parser";
             UpdateChannelMap(parser::JSON().UserParser(fVarMap.at("mq-config").as<string>(), idForParser));
-        }
-        else if (fVarMap.count("channel-config"))
-        {
+        } else if (fVarMap.count("channel-config")) {
             LOG(debug) << "channel-config: Parsing channel configuration";
             ParseChannelsFromCmdLine();
-        }
-        else
-        {
+        } else {
             LOG(warn) << "FairMQProgOptions: no channels configuration provided via neither of:";
-            for (const auto& p : fParserOptions.options())
-            {
+            for (const auto& p : fParserOptions.options()) {
                 LOG(warn) << "--" << p->canonical_display_name();
             }
         }
-    }
-    catch (exception& e)
-    {
+    } catch (exception& e) {
         LOG(error) << e.what();
         return 1;
     }
@@ -188,12 +169,9 @@ void FairMQProgOptions::ParseChannelsFromCmdLine()
     string idForParser;
 
     // check if config-key for config parser is provided
-    if (fVarMap.count("config-key"))
-    {
+    if (fVarMap.count("config-key")) {
         idForParser = fVarMap["config-key"].as<string>();
-    }
-    else if (fVarMap.count("id"))
-    {
+    } else if (fVarMap.count("id")) {
         idForParser = fVarMap["id"].as<string>();
     }
 
@@ -206,17 +184,14 @@ void FairMQProgOptions::ParseCmdLine(const int argc, char const* const* argv, bo
 
     // get options from cmd line and store in variable map
     // here we use command_line_parser instead of parse_command_line, to allow unregistered and positional options
-    if (allowUnregistered)
-    {
+    if (allowUnregistered) {
         po::command_line_parser parser{argc, argv};
         parser.options(fAllOptions).allow_unregistered();
         po::parsed_options parsed = parser.run();
         fUnregisteredOptions = po::collect_unrecognized(parsed.options, po::include_positional);
 
         po::store(parsed, fVarMap);
-    }
-    else
-    {
+    } else {
         po::store(po::parse_command_line(argc, argv, fAllOptions), fVarMap);
     }
 
@@ -229,8 +204,7 @@ void FairMQProgOptions::ParseDefaults()
 
     vector<const char*> argv(emptyArgs.size());
 
-    transform(emptyArgs.begin(), emptyArgs.end(), argv.begin(), [](const string& str)
-    {
+    transform(emptyArgs.begin(), emptyArgs.end(), argv.begin(), [](const string& str) {
         return str.c_str();
     });
 
@@ -259,8 +233,7 @@ int FairMQProgOptions::UpdateChannelMap(const unordered_map<string, vector<FairM
 void FairMQProgOptions::UpdateChannelInfo()
 {
     fChannelInfo.clear();
-    for (const auto& c : fFairMQChannelMap)
-    {
+    for (const auto& c : fFairMQChannelMap) {
         fChannelInfo.insert(make_pair(c.first, c.second.size()));
     }
 }
@@ -379,12 +352,11 @@ int FairMQProgOptions::UpdateChannelValue(const string& channelName, int index,
 
 vector<string> FairMQProgOptions::GetPropertyKeys() const
 {
-    lock_guard<mutex> lock{fConfigMutex};
+    lock_guard<mutex> lock(fMtx);
 
     vector<string> result;
 
-    for (const auto& it : fVarMap)
-    {
+    for (const auto& it : fVarMap) {
         result.push_back(it.first.c_str());
     }
 
@@ -418,8 +390,7 @@ int FairMQProgOptions::PrintOptions()
     int maxLenType = 0;
     int maxLenDefault = 0;
 
-    for (const auto& m : fVarMap)
-    {
+    for (const auto& m : fVarMap) {
         maxLenKey = max(maxLenKey, static_cast<int>(m.first.length()));
 
         VarValInfo valinfo = ConvertVariableValue<options::ToVarValInfo>()((m.second));
@@ -431,21 +402,18 @@ int FairMQProgOptions::PrintOptions()
     }
 
     // TODO : limit the value len field in a better way
-    if (maxLenValue > 100)
-    {
+    if (maxLenValue > 100) {
         maxLenValue = 100;
     }
 
-    for (const auto& o : fUnregisteredOptions)
-    {
+    for (const auto& o : fUnregisteredOptions) {
         LOG(debug) << "detected unregistered option: " << o;
     }
 
     stringstream ss;
     ss << "Configuration: \n";
 
-    for (const auto& p : mapinfo)
-    {
+    for (const auto& p : mapinfo) {
         ss << setfill(' ') << left
            << setw(maxLenKey) << p.first << " = "
            << setw(maxLenValue) << p.second.value << " "
@@ -463,11 +431,9 @@ int FairMQProgOptions::PrintOptionsRaw()
 {
     const vector<boost::shared_ptr<po::option_description>>& options = fAllOptions.options();
 
-    for (const auto& o : options)
-    {
+    for (const auto& o : options) {
         VarValInfo value;
-        if (fVarMap.count(o->canonical_display_name()))
-        {
+        if (fVarMap.count(o->canonical_display_name())) {
             value = ConvertVariableValue<options::ToVarValInfo>()((fVarMap[o->canonical_display_name()]));
         }
 
@@ -483,18 +449,14 @@ int FairMQProgOptions::PrintOptionsRaw()
 
 string FairMQProgOptions::GetStringValue(const string& key)
 {
-    unique_lock<mutex> lock(fConfigMutex);
+    lock_guard<mutex> lock(fMtx);
 
     string valueStr;
-    try
-    {
-        if (fVarMap.count(key))
-        {
+    try {
+        if (fVarMap.count(key)) {
             valueStr = ConvertVariableValue<options::ToString>()(fVarMap.at(key));
         }
-    }
-    catch (exception& e)
-    {
+    } catch (exception& e) {
         LOG(error) << "Exception thrown for the key '" << key << "'";
         LOG(error) << e.what();
     }
@@ -504,7 +466,7 @@ string FairMQProgOptions::GetStringValue(const string& key)
 
 int FairMQProgOptions::Count(const string& key) const
 {
-    unique_lock<mutex> lock(fConfigMutex);
+    lock_guard<mutex> lock(fMtx);
 
     return fVarMap.count(key);
 }
diff --git a/fairmq/options/FairMQProgOptions.h b/fairmq/options/FairMQProgOptions.h
index 1f73e72b..833452e7 100644
--- a/fairmq/options/FairMQProgOptions.h
+++ b/fairmq/options/FairMQProgOptions.h
@@ -54,17 +54,14 @@ class FairMQProgOptions
     template<typename T>
     int SetValue(const std::string& key, T val)
     {
-        std::unique_lock<std::mutex> lock(fConfigMutex);
+        std::unique_lock<std::mutex> lock(fMtx);
 
         // update variable map
         UpdateVarMap<typename std::decay<T>::type>(key, val);
 
-        if (key == "channel-config")
-        {
+        if (key == "channel-config") {
             ParseChannelsFromCmdLine();
-        }
-        else if (fChannelKeyMap.count(key))
-        {
+        } else if (fChannelKeyMap.count(key)) {
             UpdateChannelValue(fChannelKeyMap.at(key).channel, fChannelKeyMap.at(key).index, fChannelKeyMap.at(key).member, val);
         }
 
@@ -80,7 +77,7 @@ class FairMQProgOptions
     template <typename T>
     void Subscribe(const std::string& subscriber, std::function<void(typename fair::mq::PropertyChange::KeyType, T)> func)
     {
-        std::unique_lock<std::mutex> lock(fConfigMutex);
+        std::lock_guard<std::mutex> lock(fMtx);
 
         static_assert(!std::is_same<T,const char*>::value || !std::is_same<T, char*>::value,
             "In template member FairMQProgOptions::Subscribe<T>(key,Lambda) the types const char* or char* for the calback signatures are not supported.");
@@ -91,21 +88,21 @@ class FairMQProgOptions
     template <typename T>
     void Unsubscribe(const std::string& subscriber)
     {
-        std::unique_lock<std::mutex> lock(fConfigMutex);
+        std::lock_guard<std::mutex> lock(fMtx);
 
         fEvents.Unsubscribe<fair::mq::PropertyChange, T>(subscriber);
     }
 
     void SubscribeAsString(const std::string& subscriber, std::function<void(typename fair::mq::PropertyChange::KeyType, std::string)> func)
     {
-        std::unique_lock<std::mutex> lock(fConfigMutex);
+        std::lock_guard<std::mutex> lock(fMtx);
 
         fEvents.Subscribe<fair::mq::PropertyChangeAsString, std::string>(subscriber, func);
     }
 
     void UnsubscribeAsString(const std::string& subscriber)
     {
-        std::unique_lock<std::mutex> lock(fConfigMutex);
+        std::lock_guard<std::mutex> lock(fMtx);
 
         fEvents.Unsubscribe<fair::mq::PropertyChangeAsString, std::string>(subscriber);
     }
@@ -116,16 +113,13 @@ class FairMQProgOptions
     template<typename T>
     T GetValue(const std::string& key) const
     {
-        std::unique_lock<std::mutex> lock(fConfigMutex);
+        std::lock_guard<std::mutex> lock(fMtx);
 
         T val = T();
 
-        if (fVarMap.count(key))
-        {
+        if (fVarMap.count(key)) {
             val = fVarMap[key].as<T>();
-        }
-        else
-        {
+        } else {
             LOG(warn) << "Config has no key: " << key << ". Returning default constructed object.";
         }
 
@@ -137,28 +131,10 @@ class FairMQProgOptions
 
     int Count(const std::string& key) const;
 
-    template<typename T>
-    T ConvertTo(const std::string& strValue)
-    {
-        if (std::is_arithmetic<T>::value)
-        {
-            std::istringstream iss(strValue);
-            T val;
-            iss >> val;
-            return val;
-        }
-        else
-        {
-            LOG(error) << "the provided string " << strValue << " cannot be converted to the requested type. The target type must be arithmetic type.";
-        }
-    }
-
     //  add options_description
     int AddToCmdLineOptions(const boost::program_options::options_description optDesc, bool visible = true);
     boost::program_options::options_description& GetCmdLineOptions();
 
-    const boost::program_options::variables_map& GetVarMap() const { return fVarMap; }
-
     int PrintOptions();
     int PrintOptionsRaw();
 
@@ -183,7 +159,7 @@ class FairMQProgOptions
     boost::program_options::options_description fMQOptions; ///< MQ options descriptions
     boost::program_options::options_description fParserOptions; ///< MQ Parser options descriptions
 
-    mutable std::mutex fConfigMutex;
+    mutable std::mutex fMtx;
 
     std::unordered_map<std::string, int> fChannelInfo; ///< channel name - number of subchannels
     std::unordered_map<std::string, ChannelKey> fChannelKeyMap;// key=full path - val=key info
@@ -199,16 +175,6 @@ class FairMQProgOptions
     void UpdateMQValues();
     int Store(const FairMQChannelMap& channels);
 
-    template<typename T>
-    void EmitUpdate(const std::string& key, T val)
-    {
-        // compile time check whether T is const char* or char*, and in that case a compile time error is thrown.
-        static_assert(!std::is_same<T,const char*>::value || !std::is_same<T, char*>::value,
-            "In template member FairMQProgOptions::EmitUpdate<T>(key,val) the types const char* or char* for the calback signatures are not supported.");
-        fEvents.Emit<fair::mq::PropertyChange, T>(key, val);
-        fEvents.Emit<fair::mq::PropertyChangeAsString, std::string>(key, GetStringValue(key));
-    }
-
     int UpdateChannelMap(const FairMQChannelMap& map);
     template<typename T>
     int UpdateChannelValue(const std::string&, int, const std::string&, T)
diff --git a/fairmq/options/FairProgOptionsHelper.h b/fairmq/options/FairProgOptionsHelper.h
index 2d151e7a..ab984fd5 100644
--- a/fairmq/options/FairProgOptionsHelper.h
+++ b/fairmq/options/FairProgOptionsHelper.h
@@ -34,8 +34,7 @@ namespace mq
 template<class T>
 std::ostream& operator<<(std::ostream& os, const std::vector<T>& v)
 {
-    for (const auto& i : v)
-    {
+    for (const auto& i : v) {
         os << i << "  ";
     }
     return os;
@@ -105,12 +104,9 @@ struct ConvertVariableValue : T
     {
         std::string defaulted;
 
-        if (varVal.defaulted())
-        {
+        if (varVal.defaulted()) {
             defaulted = " [default]";
-        }
-        else
-        {
+        } else {
             defaulted = " [provided]";
         }
 
diff --git a/fairmq/options/runConfigEx.cxx b/fairmq/options/runConfigEx.cxx
index 2949f4d5..c92ebe51 100644
--- a/fairmq/options/runConfigEx.cxx
+++ b/fairmq/options/runConfigEx.cxx
@@ -88,12 +88,6 @@ int main(int argc, char** argv)
         MyDevice device;
         device.SetConfig(config);
 
-        // getting as string and conversion helpers
-
-        // string dataRateStr = config.GetStringValue("data-rate");
-        // double dataRate = config.ConvertTo<double>(dataRateStr);
-        // LOG(info) << "dataRate: " << dataRate;
-
         LOG(info) << "Subscribing: <string>(chans.data.0.address)";
         config.Subscribe<string>("test", [&device](const string& key, string value)
         {