
Hi Thorsten,
Section "Tutorial" ------------------------
1. I don't like that the example hardcodes strings.
Could you explain?
2. each line should have a comment about what it does.
Actually, that would duplicate explanations that are already in the tutorial. Don't you think?
3. why
po::store(po::parse_command_line(ac, av, desc), vm);
instead of
po::variables_map vm = parse_command_line( ... )
Technically, this can be done by providing an operator= which takes an instance of "parsed_options". But you'd still need 'store' for the second source.
or
vm.store( ... ) ? It would make it clearer what is stored in what.
I figured out that 'store' does not need to be member, so made it non-member. There are no other reasons.
4. po::variables_map is probably a std::map, right? I think that the way to check if an argument is a bit strange, ie,
if (vm.count("help"))
should be
if( vm.has_argument( "help" ) )
or something.
The first approach is quite idiomatic, IMO. If there's enough user pressure, 'has_option' can always be added.
5. Why is it necessary to remember the type of compresion argument? ie, could
cout << "Compression level was set to " << vm["compression"].as<int>() << ".\n";
not be written as
cout << "Compression level was set to " << vm["compression"] << ".\n";
? (if eg. a boost::variant is returned, it would just write the int)
Hmm... I did not know about operator<< for boost::variant until now. But in general, you do need to know about exact type to do any processing of the value.
6. Consider
desc.add_options() ("help", "produce help message")
why don't you have an add_options that take two and three arguments, sЕ the first empty ones can be removed?
"The first empty parenthethis"? This idea did not occur to me. I think this is doable, though again, I'd like to get more opinions.
7. In
po::value<int>(&opt)->default_value(10),
I would prefer
po::value<int>( opt, 10 ),
But then po::value<int>( 10 ), comes as obvious addition, too? I think that's rather good idea.
8.I don't find this too readable:
po::store(po::command_line_parser(ac, av). options(desc).positional(p).run(), vm);
compared to
po::command_line_parser parser(ac, av); parser.set_options( desc ); parser.set_positional_options( p ); vm.store( parser.run() );
Opinions differ ;-) I prefer the first one for compactness -- in fact I'm just emulating named parameters. Maybe when we have named parameters library in Boost the syntax can change.
9. Could this way of specifying composing()/default_value() be of any use
option += option<int>( "xx", "xx msg" ), option_with_default<int>( ""yy", var_name, 0, "yy msg" ), option_with_compose< vector<string> >( "zz", "zz msg" );
?.
I think that syntantically, it requires the same number (or even larger) number of characters. Besides, what if you want to specified both default value and 'composing' flags? options_with_default_and_compose looks too long.
10. This
po::options_description cmdline_options; cmdline_options.add(generic).add(config).add(hidden);
might be
po::options_description cmdline_options; cmdline_options += generic, config, hidden;
?
Interesting! I just found some-years-old code of mine which provides support for such initialization. It worked for vectors, like: v << 1, 2, 3, 4, 5, 6; but can be easily used for +=, too. I'm just not sure this is justified here.
11.
Have you compare with the functionality of other program option libraries? I mean, it is always nice to know that this is just the best around :-)
I've looked at several other libraries during development and still have the list somewhere. Unfortunately, no comparison is present in docs. In short, most of the libraries I've seen hardcode the type of supported options. I did not seen support for several configuration sources, either. Thanks for your comments. - Volodya