Improvement of ConfigurationReader

- renamed is_generic_options() to halt_on_generic_options(), which describes more precisely the semantic of this method
- moved halt_on_generic_options() to outside the parse_configuration_options()
- config-file processing moved from parse_configuration_options() to parse_generic_options(), because it is part of generic options
- the test "if ( vm.count(xxx) )" is no longer implicit compared to integer

diff --git a/src/config/configurationreader.cpp b/src/config/configurationreader.cpp
index 2172da6..b767709 100644 (file)
--- a/src/config/configurationreader.cpp
+++ b/src/config/configurationreader.cpp
@@ -25,7 +25,7 @@ ConfigurationReader::ConfigurationReader() :
     VersionCmdStr( "version" ),
     VersionCmdDesc( "Print the version string and exit." ),
     DaemonCmdStr( "daemon" ),
-    DaemonCmdDesc( "Run the program as a deamon." ),
+    DaemonCmdDesc( "Run the evil in background." ),
     DefaultConfigFileName( "pingcheck.conf" ),
     ConfigFileCmdStr( "config-file" ),
     ConfigFileCmdDesc( "Name of the configuration file." ),
@@ -65,16 +65,30 @@ bool ConfigurationReader::parse(
     BOOST_ASSERT( argv != NULL );
 
     variables_map vm;
+
     bool command_line_processed = process_command_line( argc, argv, vm );
+    if ( command_line_processed )
+    {
+        parse_generic_options( vm );
+
+        bool terminate_app = halt_on_generic_options( vm );
+        if ( terminate_app )
+        {
+            return false;
+        }
+    }
+
     bool configuration_file_processed = process_configuration_file( vm );
 
     bool input_processed = command_line_processed && configuration_file_processed;
-    if (input_processed)
+    if ( input_processed )
     {
         return parse_configuration_options( vm );
     }
-
-    return false;
+    else
+    {
+        return false;
+    }
 }
 
 Configuration ConfigurationReader::get_configuration() const
@@ -82,6 +96,15 @@ Configuration ConfigurationReader::get_configuration() const
     return Config;
 }
 
+bool ConfigurationReader::halt_on_generic_options( const variables_map &vm ) const
+{
+    bool is_help = ( vm.count( HelpCmdStr ) > 0 );
+    bool is_version = ( vm.count( VersionCmdStr ) > 0 );
+    bool terminate_app = is_help || is_version;
+
+    return terminate_app;
+}
+
 options_description ConfigurationReader::get_generic_options() const
 {
     options_description options( "Generic options" );
@@ -95,39 +118,42 @@ options_description ConfigurationReader::get_generic_options() const
     return options;
 }
 
-bool ConfigurationReader::is_generic_options( const variables_map &vm ) const
-{
-    bool is_help = ( vm.count( HelpCmdStr ) > 0 );
-    bool is_version = ( vm.count( VersionCmdStr ) > 0 );
-    bool is_daemon = ( vm.count( DaemonCmdStr ) > 0 );
-    bool is_a_generic_option = is_help || is_version || is_daemon;
-
-    return is_a_generic_option;
-}
-
 bool ConfigurationReader::parse_generic_options(
-        const variables_map& vm,
-        const options_description& visible_options
+        const variables_map& vm
 )
 {
-    // TODO how to make clear that returning false makes the application abort
-    // and returning true lets it continue processing the configuration file?
-    if ( vm.count( HelpCmdStr ) )
+    // help
+    if ( vm.count( HelpCmdStr ) > 0)
     {
+        options_description generic = get_generic_options();
+        options_description config = get_configuration_options();
+
+        options_description visible_options( "Allowed options" );
+        visible_options.add( generic ).add( config );
+
         cout << visible_options << endl;
-        return false;
+        return true;
     }
 
-    if ( vm.count( VersionCmdStr ) )
+    // version
+    if ( vm.count( VersionCmdStr ) > 0 )
     {
         cout << PROJECT_NAME << " version " << VERSION_STRING << endl;
-        return false;
+        return true;
     }
 
-    if ( vm.count( DaemonCmdStr ) )
+    // daemon
+    bool have_daemon = ( vm.count( DaemonCmdStr ) > 0 );
+    Config.set_daemon( have_daemon );
+    cout << DaemonCmdStr << "=" << have_daemon << endl;
+
+    // config-file
+    if ( vm.count( ConfigFileCmdStr ) > 0 )
     {
-        Config.set_daemon( true );
-        return true;
+        string config_file_name = vm[ ConfigFileCmdStr ].as<string> ();
+        Config.set_config_file_name( config_file_name );
+
+        cout << ConfigFileCmdStr << "=" << config_file_name << endl;
     }
 
     return false;
@@ -152,17 +178,8 @@ options_description ConfigurationReader::get_configuration_options() const
 
 bool ConfigurationReader::parse_configuration_options( const variables_map &vm )
 {
-    // config-file
-    if ( vm.count( ConfigFileCmdStr ) )
-    {
-        string config_file_name = vm[ ConfigFileCmdStr ].as<string> ();
-        Config.set_config_file_name( config_file_name );
-
-        cout << ConfigFileCmdStr << "=" << config_file_name << endl;
-    }
-
     // source-network-interface
-    if ( vm.count( SourceNetworkInterfaceCmdStr ) )
+    if ( vm.count( SourceNetworkInterfaceCmdStr ) > 0 )
     {
         string source_network_interface =
                 vm[ SourceNetworkInterfaceCmdStr ].as<string> ();
@@ -173,7 +190,7 @@ bool ConfigurationReader::parse_configuration_options( const variables_map &vm )
     }
 
     // nameserver
-    if ( vm.count( NameServerCmdStr ) )
+    if ( vm.count( NameServerCmdStr ) > 0 )
     {
         string nameserver = vm[ NameServerCmdStr ].as<string> ();
         Config.set_nameserver( nameserver );
@@ -183,7 +200,7 @@ bool ConfigurationReader::parse_configuration_options( const variables_map &vm )
 
     // hosts-down-limit
     int host_down_limit = 0;
-    if ( vm.count( HostsDownLimitCmdStr ) )
+    if ( vm.count( HostsDownLimitCmdStr ) > 0 )
     {
         host_down_limit = vm[ HostsDownLimitCmdStr ].as<int> ();
         Config.set_hosts_down_limit( host_down_limit );
@@ -192,7 +209,7 @@ bool ConfigurationReader::parse_configuration_options( const variables_map &vm )
     }
 
     // ping-fail-limit
-    if ( vm.count( PingFailLimitCmdStr ) )
+    if ( vm.count( PingFailLimitCmdStr ) > 0 )
     {
         int ping_fail_limit = vm[ PingFailLimitCmdStr ].as<int> ();
         Config.set_ping_fail_limit( ping_fail_limit );
@@ -201,7 +218,7 @@ bool ConfigurationReader::parse_configuration_options( const variables_map &vm )
     }
 
     // status-notifier-cmd
-    if ( vm.count( StatusNotifierCmdCmdStr ) )
+    if ( vm.count( StatusNotifierCmdCmdStr ) > 0 )
     {
         string status_notifier_cmd = vm[ StatusNotifierCmdCmdStr ].as<string>();
         Config.set_status_notifier_cmd( status_notifier_cmd );
@@ -210,7 +227,7 @@ bool ConfigurationReader::parse_configuration_options( const variables_map &vm )
     }
 
     // link-up-interval
-    if ( vm.count( LinkUpIntervalCmdStr ) )
+    if ( vm.count( LinkUpIntervalCmdStr ) > 0 )
     {
         int link_up_interval_in_min = vm[ LinkUpIntervalCmdStr ].as<int>();
         Config.set_link_up_interval_in_min( link_up_interval_in_min );
@@ -221,7 +238,7 @@ bool ConfigurationReader::parse_configuration_options( const variables_map &vm )
 
     // [host] name
     size_t hosts_names_count = 0;
-    if ( vm.count( HostNameCmdStr ) )
+    if ( vm.count( HostNameCmdStr ) > 0 )
     {
         vector<HostItem> hosts_list;
 
@@ -243,7 +260,7 @@ bool ConfigurationReader::parse_configuration_options( const variables_map &vm )
 
     // [host] interval
     size_t hosts_interval_count = 0;
-    if ( vm.count( HostIntervalCmdStr ) )
+    if ( vm.count( HostIntervalCmdStr ) > 0 )
     {
         vector<HostItem> hosts_list = Config.get_hosts();
         vector<HostItem>::iterator hosts_it = hosts_list.begin();
@@ -282,9 +299,6 @@ bool ConfigurationReader::process_command_line(
         options_description cmdline_options;
         cmdline_options.add( generic ).add( config );
 
-        options_description visible( "Allowed options" );
-        visible.add( generic ).add( config );
-
         positional_options_description p;
         (void) p.add( HostNameCmdStr.c_str(), -1 );
 
@@ -293,19 +307,13 @@ bool ConfigurationReader::process_command_line(
         store( parsed_opt, vm );
         notify( vm );
 
-        if ( is_generic_options( vm ) )
-        {
-            return parse_generic_options( vm, visible );
-        }
-
+        return true;
     }
     catch ( const std::exception &ex )
     {
         cerr << ex.what() << endl;
         return false;
     }
-
-    return true;
 }
 
 bool ConfigurationReader::process_configuration_file( variables_map &vm )

diff --git a/src/config/configurationreader.h b/src/config/configurationreader.h
index 1206453..f7b8e2a 100644 (file)
--- a/src/config/configurationreader.h
+++ b/src/config/configurationreader.h
@@ -24,13 +24,12 @@ public:
     Configuration get_configuration() const;
 
 private:
-    boost::program_options::options_description get_generic_options() const;
-    bool is_generic_options(
+    bool halt_on_generic_options(
             const boost::program_options::variables_map &vm
     ) const;
+    boost::program_options::options_description get_generic_options() const;
     bool parse_generic_options(
-            const boost::program_options::variables_map &vm,
-            const boost::program_options::options_description &visible_options
+            const boost::program_options::variables_map &vm
     );
 
     boost::program_options::options_description get_configuration_options() const;