Two days ago I was asked to have a look at a nonworking piece of code, based on boost::program_options, which went like this:

options_description desc("Usage");
desc.add_options()
    ("host,h", value<string>(), "The host.")
    ("file,f", value<string>(), "This is the file.");
    ("mode,m", value<string>(), "The mode.");

If you don’t recognise the syntax, know that add_options() method of options_description returns a builder object (options_description_easy_init), with an operator () taking three parameters, which calls the add method on the original options_description object, and returns the builder object itself. This gives us the ability to use this unusual syntax of chained parentheses to add a sequence of arguments to the options_description object.

There is a bug, can you spot it?

Visual Studio 2013 didn’t complain while compiling that code and for a reason (at least not at the default warning level, 3). The description text was way longer, so the problem was harder to see, and indeed I didn’t spot it right away. But then, while debugging line by line, I’ve noticed that the desc.add_options() line was executed in two steps, rather than one, meaning that there were two statements. This allowed me to realise where the bug was.

Can you see it now?

The “file,f” parameter descriptor has a semicolon at the end. This results in the last line being a parenthesised use of the comma operator and not a call to options_description_easy_init::operator ()(…) as we thought. If you don’t know what the comma operator is, have a look at the ugliness of the examples contained in the wikipedia page.

options_description desc("Usage");
desc.add_options()
    ("host,h", value<string>(), "The host.")
    ("file,f", value<string>(), "This is the file.") // no semicolon here!!!
    ("mode,m", value<string>(), "The mode.");

At a higher warning level, some compilers would have complained about the first operand not having side-effects (being it a constant), but this warning is not generated in other uses of the same syntax (e.g. if the first parameter name was not a constant, but was returned by a function). You can’t even count on warnings, since that is a perfectly reasonable syntax for something else.

This syntax is not only unusual, but also dangerous. What the boost::program_options library designers could have done to avoid with the funny syntax was a chain of methods:

options_description desc("Usage");
desc.add("host,h", value<string>(), "The host.")
    .add("file,f", value<string>(), "This is the file.")
    .add("mode,m", value<string>(), "The mode.");

This code is as readable as the original code, even if slightly more verbose in some cases (not this one, which is actually two characters shorter). On the other hand, the latter syntax is definitely less error-prone.

PS: Can someone help me finding a good use of the comma operator in C++11

(2015/01/29 19:54 CET) Edited clarifying that the solution is a proposal, the actual code with the current version of the library is much more verbose.