patterncppMinor
Program that handles program options
Viewed 0 times
programhandlesthatoptions
Problem
I'm using
How can I reduce the number of allocations in this program?
Here's an example of what valgrind reports:
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
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 allocatedAnd 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
You should also consider splitting your code in different methods and make sure that everything has a single concern. Your
What about moving your
I'd also introduce a
Your
Execute next should follow the same idea and be split in a few methods like:
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.