program options from command line initialize

Question

I'd like to receive a CR for the following command-line program options' initializing using boost::program_options. I'd like to know if there is a better way of doing this using boost::program_options or using another available library/code:

#include <iostream>
#include <boost/program_options.hpp>

using namespace std;
namespace po = boost::program_options;

int main(int ac, char* av[]) {
    /// Command line options initialize
    po::options_description visible_desc("Usage: program [options] [path/]logger_filename");
    po::options_description full_desc;
    po::positional_options_description pd;
    bool verbose, anomaly_detection, analyze_activity;
    string normal_login_word;
    string log_file_path;
    string week_start_day;

    auto generic_group = po::options_description("Generic options");
    generic_group.add_options()
            ("help,h", "produce help message")
            ("verbose", po::value<bool>(&verbose)->default_value(false), "Show detailed times of login.")
            ;

    auto calender_group = po::options_description("Calender options");
    calender_group.add_options()
            ("week-start-day,d", po::value<string>(&week_start_day)->default_value("Monday"), "Week starting day ('--week-start-day help' for a list).")
            ;

    auto log_group = po::options_description("Logger options");
    log_group.add_options()
            ;

    auto hidden_options_group = po::options_description("Logger options");
    hidden_options_group.add_options()
            ("log-path,l", po::value<string>(&log_file_path)->default_value("/home/sherlock/message_from_computer"), "Path to login/logout logger.")
            ;

    auto anomalies_group = po::options_description("Mode options");
    anomalies_group.add_options()
            ("analyze-log", po::value<bool>(&analyze_activity)->default_value(true), "Analyze activity - show activity times and summarise activity.")
            ("anomaly-detection", po::value<bool>(&anomaly_detection)->default_value(false), "Check for anomalies in logger.")
            ("normal-login-word", po::value<string>(&normal_login_word)->default_value("login"), "For anomaly detector- word that should symbol a login line in login/logout logger (after '+' sign).")
             ;

    pd.add("log-path", -1);

    visible_desc.add(generic_group).add(calender_group).add(log_group).add(anomalies_group);
    full_desc.add(generic_group).add(calender_group).add(log_group).add(anomalies_group).add(hidden_options_group);

    /// Command line options applying
    po::variables_map vm;
    po::store(
            po::command_line_parser(ac, av)
            .options(full_desc)
            .positional(pd)
            .run(), vm);
    po::notify(vm);

    /*
     ... Analyze options ...
    */
    return 0;
}

---

Update:

after @pacmaninbw and @ALX23z CRs I created a new updated post: program options from command line initialize [v2 - after CR]

Answer 1

In addition to the command line, there are at least two other methods to pass this information in: the .ini file was mentioned in another answer, environment variables are also an option. It's best not to force the user to type in too many options for the command line.

Avoid Using Namespace std

If you are coding professionally, you probably should get out of the habit of using the using namespace std; statement. The code will more clearly define where cout and other identifiers are coming from (std::cin, std::cout). As you start using namespaces in your code it is better to identify where each function comes from because there may be function name collisions from different namespaces. The function cout you may override within your own classes. This Stack Overflow question discusses this in more detail.

Complexity

The function main() is too complex (does too much). As programs grow in size the use of main() should be limited to calling functions that parse the command line, calling functions that set up for processing, calling functions that execute the desired function of the program, and calling functions to clean up after the main portion of the program.

There is also a programming principle called the Single Responsibility Principle that applies here. The Single Responsibility Principle states:

that every module, class, or function should have responsibility over a single part of the functionality provided by the software, and that responsibility should be entirely encapsulated by that module, class or function.

The code to set up the command line options should be in a function of its own. That function should probably call multiple functions to set up the options: one function for each of the option descriptions created.

For a program that is going to be this complex it might be good to #include <cstdlib> and use EXIT_SUCCESS and EXIT_FAILURE to make the code more self-documenting.

Variable Names and Declarations

The variable name pd is not very descriptive; someone who had to maintain the code would not be able to do it easily.

For the same reason using po rather than boost::program_options:: could make the program a lot harder to maintain. A maintainer would have to search through the program to find out what po is.

When declaring variables such as verbose, anomaly_detection and analyze_activity it would be better to declare each variable on a separate line and possibly initialize them at the same time. This would make it easier to add more variables and to find the variable declarations.

    bool verbose = true;
    bool anomaly_detection = false;
    bool analyze_activity = true;

Answer 2

I tried boost::program_options but simply didn't like it.

boost::program_options has the feature that certain options are additive or hidden - which is why their usage requires so much extra work (you have to specify each option you use and state how they are processed). If you are fine without these features, you can work with config files - *.ini format - and simply read and store all option that the file has, no need for the whole mess in the main. It is not hard to implement a config file reader and Windows has built in functions for reading arguments from a .ini file.

Also this way you can supply input parameters via an .ini file instead of command line, which is much more convenient if you have more than 5 parameters and I tend to have dozens in tests.

In my case, I implemented a class ConfigFile that basically wraps a std::map<string,string> whose keys are in format SECTION/NAME; with added functions for getting/setting int/double/string values as well as functionality for reading a whole ini file.

In case you want to read a command line via the ConfigFile class: you can reinterpret cmd arguments and store them inside the ConfigFile class. string-based options simply store as is or optionally add a prefix-section. To support index based options you ought to provide a vector of strings to that interprets them.

I don't know of any open-source library that implements something like this, though I didn't check for any. But it is easy to implement on your own. I might publish my implementation together with a couple of other utility features as an open-source but not now. So...

Example of CMD parsing:

class CConfig
{
public:
            // sKeys is used to store index based inputs of CMD
    void ParseCMD(  int                         argc,
            const   char**                      argv,
                    std::vector<std::string>    sKeys = {});

   std::string  GetString(std::string key,  std::string  defaultValue);
   double       GetDouble(std::string key,  double       defaultValue);
   int          GetInt(   std::string key,  int          defaultValue);

   void         SetString(std::string key,  std::string  value);
   void         SetDouble(std::string key,  double       value);
   void         SetInt(   std::string key,  int          value);

private:
    std::mutex                          m_mutex;
    std::map<std::string, std::string>  m_map;
};

void CConfig::ParseCMD( int                         argc,
                const   char**                      argv,
                        std::vector<std::string>    sKeys)
    {
        std::lock_guard<std::mutex> g(m_mutex);
        size_t index = 0;
        for (int i = 1; i < argc; i++)
        {
            if (argv[i][0] != '-')
            {
                if (sKeys.size() <= index)
                {
                    std::cout "\n[warning]: unassigned input parameters; total number of index-based input parameters is: " << sKeys.size();
                    continue;
                }

                m_map[sKeys[index]] = argv[i][0];
                continue;
            }

            if (i + 1 < argc && argv[i + 1][0] != '-')
            {
                m_map[argv[i]+1] = argv[i + 1];
                i++;
            }
            else
            {
                // simply defines this key with empty value
                m_map[argv[i]+1] = {};
            }
        }

        if (sKeys.size() > index)
        {
            // or add option for making it into an error and not just a warning print
            std::cout << "\n[warning]: not all index-based values were assigned");
        }
    }

// in main simply write:
   int main(int argc, const char* argv[])
   {
         CConfig config;
         config.ParseCMD(argc, argv, {/*list of strings for identification*/});
         ...
   }