Program that handles program options

Question

I'm using boost::program_options to parse input from the user. However, while the standard library makes this an easy task to do, it's causing an (in my opinion) excessive amount of allocations. Of course, for such a small program on modern systems, it probably isn't that big of a deal, but I would like to nip the problem in the bud before it spirals out of control.

How can I reduce the number of allocations in this program?

Here's an example of what valgrind reports:

 $ test
Compression level was not set.
 $ test --help
Allowed options:
 --help produce help message
 --compression arg set compression level
 $ test --compression 10
Compression level was set to 10.
 $ test --compression 5
Compression level was set to 5.
 $ test --compressio 
error: the required argument for option '--compression' is missing
total heap usage: 266 allocs, 266 frees, 11,430 bytes allocated

And the code dump:

#include <algorithm>
#include <iostream>
#include <iterator>
#include <string>
#include <vector>
#include <memory>
#include <sstream>
#include <cstring>
#include <boost/program_options.hpp>
#include <boost/iterator/transform_iterator.hpp>
namespace po = boost::program_options;
// "std::vector<std::string> to char* array"
// http://stackoverflow.com/a/7048972/3920237
const char *convert_to_cstr(const std::string & s)
{
 return s.c_str(); 
}
int main(int argc, char* argv[])
{
 po::options_description desc("Allowed options");
 desc.add_options()
 ("help", "produce help message")
 ("compression", po::value<int>(), "set compression level")
 ;
 std::string input = "";
 const std::string prompt = " $ ";
 std::vector<std::string> args;
 std::istringstream iss;
 std::string token;
 po::variables_map vm; 
 while (true)
 {
 try {
 std::cout << prompt;
 std::getline(std::cin, input);
 if (input == "quit")
 break;
 iss.clear();
 iss.str(input);
 args.clear();
 while (iss >> token)
 {
 args.push_back(token); 
 }
 auto beg = boost::make_transform_iterator(args.begin(), 
 convert_to_cstr);
 auto end = boost::make_transform_iterator(args.end(), 
 convert_to_cstr);
 const std::vector<const char*> vc { beg, end };
 /* std::transform(args.begin(), args.end(), std::back_inserter(vc), convert); */
 vm.clear();
 po::store(po::parse_command_line(vc.size(), vc.data(), desc), vm);
 po::notify(vm); 
 if (vm.count("help")) {
 std::cout << desc << "\n";
 continue;
 }
 if (vm.count("compression")) {
 std::cout << "Compression level was set to " 
 << vm["compression"].as<int>() << ".\n";
 } else {
 std::cout << "Compression level was not set.\n";
 }
 }
 catch(std::exception& e) {
 std::cerr << "error: " << e.what() << "\n";
 continue;
 }
 catch(...) {
 std::cerr << "Exception of unknown type!\n";
 } 
 }
 return 0; 
}

• glampert: Your concerns are legitimate for hight perf software. If that is the case, then I would use getopt instead. It is a very ugly C function that messes with globals, but it is also very fast and doesn't allocate memory. Once your wrap it inside a class or helper function, it isn't that bad though.
• utnapistim: This sounds like a case of premature optimization (unless you have concrete requirements for a resource-starved environment or performance-critical application). To keep the code from spiraling out of control, refactor into smaller code units, by responsibility. Memory allocations shouldn't matter (unless they concretely cause a problem).

Answer 1

I think you should not really need to worry about that. This is for sure a non-performance critical part of your application and all the objects you create are almost immediately freed. My advice is to avoid premature optimisation and to focus on code readability instead.

I'd use some meaningful names. I'd replace iss, beg, vm with inputStream, beginning, variables_map. It improves readability a lot.

You should also consider splitting your code in different methods and make sure that everything has a single concern. Your main is doing everything and that's not very good in my opinion. What about moving your prompt declaration to a constant defined at class level instead that inside the main? I'd also introduce a create_options_descriptions() and a bool execute_next_command(...) methods.

Your main should look like:

int main(int argc, char* argv[])
{
 po::options_description options_description = create_options_description();
 po::variables_map vm; 
 /*
 * It's better to pass std::cin and std::cout as dependencies so it
 * will be trivial to change it with references to other streams
 * if you ever need to.
 */
 while(execute_next_command(std::cin, std::cout, vm));
}

Execute next should follow the same idea and be split in a few methods like: read_input(...) that produces a list of arguments and bool process_args(...) that will take care of processing them and do the appropriate thing.

• user53899: Actually, I think that po::options_description options_description = create_options_description(); won't work. See my question posted here.
• mariosangiorgio: I'll have a look, but unfortunately I'm not a C++ expert at all and I've never used boost::program_options in any project.
• user53899: Nevermind, I remembered that copying is fine, copy assignment seems to be the core of the issue.