From: Petr Koèmíd (pkocmid_at_[hidden])
Date: 2003-05-26 20:47:45
On Wednesday 21 of May 2003 13:59, Aleksey Gurtovoy wrote:
> The formal review of Vladimir Prus' Command Line & Config
> ("program_options") library starts today, May 21, and will continue until
> Monday, June 2nd. I will be serving as review manager.
I take a deep look down to the code, not bothering with documentation nor
actually testing it. I am looking at the submission packaged at 23.5.2003.
I consider hardcoding of std:string throughout CL&CL to be a major design
flaw. As it is, it's virtually nonportable to platforms/applications which
could use unicode and/or platform using specific startup instead of C legacy
main(). (Consider: Win32 GUI applications, Linux running unicode console,
filesystems with uniceode file names/content, embedded devices).
Why not using templates for strings with std:string as a default type? Some
people are tied by API to slightly different string model, such as Qt or MFC
strings (those mentioned are, or are not, depending on library mixture,
compatible with std:string), with command line already filtered and passed
down to some application object (no access to naked argv) and so on.
Decoupling parsing from doing actual stream input would increase reusability.
Spirit does parsing with iterators only, why not borrow some declarative
idioms or actual code from it? It would be more interesting to have an
option/additional parser registration at compile time instead of at run-time
since program options/configurations are rarely of dynamic nature.
Also, I'd suggest to add some genericity on value retriving mechanism, at
least at the interface level (e.g. replacing config_file with more abstract
config_storage concept, consider: windows registry API, chip card reader API,
shared memory, XML, ...).
Actually, by it's interface a config_file class mixes together three different
concepts: storage accessor concept (input stream with ownership management),
content grammar definition concept (parser rulebase managed by add_option)
and result-data-container-with-integrated-iterator concept (with rather
magical operator++, name() and value(), why not simple get() returning tuple?
Or get() taking a key argument?)
Such a mix of concepts in one class is confusing and prevents extensibility of
all mechanisms. More, private implementation stuff should be not part of the
interface in boost-quality code.
Ability of storing configuration values back to persistant storage would be a
natural outcome of correct solution to 3.
Well, what I like about CL&CL is the pretty idea of domain-specific option
definition language, but I'd rather see a compile time solution of it. I
can't resist to say: "Spirit rules!" ;-)
I believe if CL&CL is supposed to be an universal library, it should be
redesigned adding some more versatility layers before its acceptation into
Boost list run by bdawes at acm.org, gregod at cs.rpi.edu, cpdaniel at pacbell.net, john at johnmaddock.co.uk