HiveBrain v1.2.0
Get Started
← Back to all entries
patterncppMinor

Program that handles program options

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
programhandlesthatoptions

Problem

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
#include
#include
#include
#include
#include
#include

#include

#include
#include
namespace po = boost::program_options;

// "std::vector 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(), "set compression level")
;

std::string input = "";

const std::string prompt = " $ ";

std::vector args;
std::istringstream iss;

std::string token;

po::variables_map vm;
while (true)
{
try {
std::cout > 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::vec

Solution

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.

Code Snippets

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));
}

Context

StackExchange Code Review Q#64014, answer score: 5

Revisions (0)

No revisions yet.