Boost logo

Boost :

From: Beman Dawes (bdawes_at_[hidden])
Date: 2003-06-02 11:37:11

This review is based purely on reading the documentation. The code was not
inspected and no tests were run. I also skim read some of the other review

In general, I like the library and think that it should be accepted by
Boost. But there are a number of issues, and I have the feeling that
careful revision based on review comments would pay big dividends.

Thanks to Volodya for such a useful submission!


Design Issues

* As someone else already mentioned, cmdline and config_file might be
easier to use by STL-experienced programmers if they more clearly modelled
containers (with separate iterator types), or else followed the
options_and_arguments usage, which returns a vector<> ref that can be
iterated over. The current design seems to mix typical container operations
with typical iterator operations.

* boost::program_options::options_and_arguments - the difference between
the get_values() and get_all_values() members isn't clear. Not sure if this
is just a doc issue, or there is an underlying design confusion.

* wchar_t and UDT-char support. These are probably important long-term
goals, but IMO should be deferred until more experience has built up with
the basic narrow-char design.


* Does a programmer have to be aware of a distinction between command line
args and config file args in normal usage? (This may be a documentation
issue rather than a design issue. Perhaps "Design overview" section (1)
dealing with cmdline and config_file needs an added sentence like "Unless a
user wishes to explicitly control parsing, the use of these parser classes
is hidden from the programmer.")

* Are common forms of command line indirection to a config file
supported? Like @pathname, for example? [Later... it really doesn't look
like cmdline does indirection. If not, that's a serious oversight, IMO.
Easy to fix, however.]

* Can indirection be recursive? (@file1 contains an indirection like
@file2.) If so, are cycles detected?

* How do you code a comment in a config file? (Comments in config files are
really important - this feature needs to be added if not already

* Is there a function or class which does the argc/argv replacement-trick
for command line indirection? It is so easy (one #include + one line of
code) even for existing programs that it might be valuable as a standalone
feature even if built-in to the cmdline parser itself.

Minor Issues and Quibbles

* boost::program_options::options_and_arguments member names: I'd prefer
just plain "value", "values", and "all_values". The "get_" adds little and
isn't usual Boost or Standard Library practice.

* The C++ standard, 3.6.1, specifies the declaration of main as "int
main(int argc, char* argv[])". Your options_description.html,
custom_syntax.html, and cmdline.html examples uses "int main(int ac, const
char **av)", and that will cause problems with some compilers, IIRC.

* Is there a style for +foo -bar style arguments where "+" yields a value
of true and "-" yields a value of false?


* Need work to make them more consistent with usual Boost practice. Logo,
etc. I tried very hard to get to like the doc style, but just plain feel it
isn't as good as the traditional Boost approaches.

* Grammar and English usage are a bit awkward in places. I'll volunteer to
do a pass over the docs once in final form.

* The "simple usage" link points to a section entitled "Options
description", which seems a misnomer since what is there is in fact a nice

* The options_description.html example would be much improved by showing
several command line invocations with different arguments together with the
output that each produced.

* At first glance the options_description.html example's try block contains
code that I would have guessed should be outside the try block. Is this
ever explained?

* Acknowledgements section missing.

* Some of the filenames are too long:

* classboost_1_1program__options_1_1cmdline.html mentions operator++
several times, but there is no public member of that name, and no public
members which might return an iterator.

* classboost_1_1program__options_1_1cmdline.html/Member Function
Documentation/Parameters refers to "'next' member function", but there
isn't any member function named 'next'.

* classboost_1_1program__options_1_1options__and__arguments.html says that
the "unsigned count(const std::string & name) member "Checks if option is
present". Should that be "Returns the number of options present named

* classboost_1_1program__options_1_1cmdline.html style_t description would
be a lot clearer if given in a table with columns name and meaning, and one
row per style. There isn't any need to see the actual values assigned each

--- end ---

Boost list run by bdawes at, gregod at, cpdaniel at, john at