Command line arguments adhering Posix and GNU standards

Question

The following code extracts command line parameters and modifies argc and argv parameters of the. After all parameters are read remaining values (especially after --) are remaining.

Please keep following questions in mind while reviewing:

• Is it good practice to modify argc and argv?
• How to make clear to the caller they change?
• Are the bool and std::string specialisations obvious?
• Is there a shorter/compacter way than using the currently used mixin?

Condensed:

/**
* Basic functions shared by general template and specialisations
*/
template <typename T>
class CliArgMixin
{
protected:
    /**
    * Helper function to delete arguments.
    */
    static void deleteArg(int& argc, char* argv[], int pos)
    {
        assert(argv);
        assert(argc > 0);

        if(pos >= argc) return;

        for(int i=pos; i<argc-1; ++i)
        {
            argv[i] = argv[i+1];
        }

        argv[argc] = 0;
        argc--;
        argv[argc] = 0;
    }

    /**
    * Search by an alternative argument name.
    * Use this to use long and short arguments e.g. '-l 10' and '--list 10'
    */
    bool search(int& argc, char* argv[], const std::string& name)
    {
        assert(argv);
        assert(argc > 0);

        if(name.empty()) return false;

        std::string entry;
        for(int i=0; i<argc; ++i)
        {
            assert(argv[i]);
            if(argv[i][0] == '-' && argv[i][1] != '-')
            {
                entry.assign(argv[i] + 1);
            }
            else if(argv[i][0] == '-' && argv[i][1] == '-')
            {
                // skip on --
                if(argv[i][2] == 0) return false;
                entry.assign(argv[i] + 2);
            }
            else
            {
                continue;
            }

            if(entry == name)
            {
                extract(argc, argv, i);
                m_extracted = true;
                return true;
            }
        }
        return false;
    }

    /**
    * Search condesed POSIX short options like '-x12'
    */
    void searchCondensed(int& argc, char* argv[], const std::string& name)
    {
        assert(argc > 0);
        assert(argv);
        if(name.empty()) return;

        std::string entry;
        for(int i=0; i<argc; ++i)
        {
            if(argv[i][0] == '-' && argv[i][1] != '-')
            {
                entry = argv[i][1];
            }
            else if(argv[i][0] == '-' && argv[i][1] == '-')
            {
                // skip on --
                if(argv[i][2] == 0) return;
            }
            else
            {
                continue;
            }

            if(entry == name)
            {
                extractCondensed(argc, argv, i);
                m_extracted = true;
                return;
            }
        }
    }

    /**
    * Returns true if an argument value from the command line was extracted
    * false otherwise.
    */
    bool extractedValue() const { return m_extracted; }

    CliArgMixin(const T& dflt)
    : m_value(dflt)
    , m_extracted(false)
    {}

    virtual ~CliArgMixin() {}

    virtual void extract(int& argc, char* argv[], int index) = 0;
    virtual void extractCondensed(int& argc, char* argv[], int index) = 0;

    T       m_value;
    bool    m_extracted;
};

/**
* General template.
*/
template <typename T>
class CliArg : public CliArgMixin<T>
{
    typedef CliArgMixin<T> Mixin;
public:
    CliArg(int& argc, char* argv[], const std::string& opt, const std::string& longOpt, const T& d = T())
    : Mixin(d)
    {
        if(opt.length() > 1)
        {
            throw std::invalid_argument("Short option must be one character or empty");
        }

        if(longOpt.length() == 1)
        {
            throw std::invalid_argument("Long option must not be one character");
        }

        if( Mixin::search(argc, argv, longOpt) == false )
        {
            if( Mixin::search(argc, argv, opt) == false )
            {
                Mixin::searchCondensed(argc, argv, opt);
            }
        }
    }

    ~CliArg() {}

    const T& value() const { return CliArgMixin<T>::m_value; }
    operator const T&() const { return CliArgMixin<T>::m_value; }

private:
    CliArg() = delete;

    virtual void extract(int& argc, char* argv[], int index)
    {
        if(index+1 < argc)
        {
            std::istringstream(argv[index+1]) >> Mixin::m_value;
            Mixin::deleteArg(argc, argv, index);
            // TRICKY: after the i-th was a delete index is now the old i+1;
            Mixin::deleteArg(argc, argv, index);
        }
        else
        {
            std::ostringstream msg;
            msg << "Missing value for parameter: " << argv[index];
            throw std::runtime_error( msg.str() );
        }
    }

    virtual void extractCondensed(int& argc, char** argv, int index)
    {
        // skip '-' and opt
        std::istringstream(argv[index]+2) >> Mixin::m_value;
        Mixin::deleteArg(argc, argv, index);
    }
};

/**
* Specialisation for std::string values
*/
template <>
class CliArg< std::string > : public CliArgMixin< std::string >
{
    typedef CliArgMixin<std::string> Mixin;
public:
    CliArg(int& argc, char* argv[], const std::string& opt, const std::string& longOpt, const std::string& d = "")
    : Mixin(d)
    {
        if(opt.length() > 1)
        {
            throw std::invalid_argument("Short option must be one character or empty");
        }

        if(longOpt.length() == 1)
        {
            throw std::invalid_argument("Long option must not be one character");
        }

        if( Mixin::search(argc, argv, longOpt) == false)
        {
            if( Mixin::search(argc, argv, opt) == false)
            {
                Mixin::searchCondensed(argc, argv, opt);
            }
        }
    }

    ~CliArg() {}

    const std::string& value() const { return Mixin::m_value; }
    operator const std::string&() const  { return Mixin::m_value; }

private:
    CliArg() = delete;

    virtual void extract(int& argc, char** argv, int index)
    {
        if(index+1 < argc)
        {
            Mixin::m_value = argv[index+1];
            Mixin::deleteArg(argc, argv, index);
            // TRICKY: after the i-th was a delete i is now the old i+1;
            Mixin::deleteArg(argc, argv, index);
        }
        else
        {
            std::ostringstream msg;
            msg << "Missing value for parameter: " << argv[index];
            throw std::runtime_error( msg.str() );
        }
    }
    virtual void extractCondensed(int& argc, char** argv, int index)
    {
        // skip '-' and opt
        Mixin::m_value = argv[index]+2;
        Mixin::deleteArg(argc, argv, index);
    }
};

/**
* Specialisation for bool flags with no following value.
*/
template <>
class CliArg< bool > : public CliArgMixin< bool >
{
    typedef CliArgMixin< bool > Mixin;
public:
    CliArg(int& argc, char* argv[], const std::string& opt, const std::string& longOpt)
    : Mixin(false)
    {
        assert(argc > 0);
        assert(argv);
        if(opt.length() > 1)
        {
            throw std::invalid_argument("Short option must be one character or empty");
        }

        if(longOpt.length() == 1)
        {
            throw std::invalid_argument("Long option must not be one character");
        }

        if( Mixin::search(argc, argv, longOpt) == false )
        {
            if( Mixin::search(argc, argv, opt) == false )
            {
                searchCondensed(argc, argv, opt);
            }
        }
    }

    ~CliArg() {}

    bool value() const { return Mixin::m_value; }

    operator bool() const  { return Mixin::m_value; }

private:

    CliArg() = delete;

    virtual void extract(int& argc, char** argv, int index)
    {
        Mixin::m_value = true;
        Mixin::deleteArg(argc, argv, index);
    }

    /**
    * There are no condensed value for short options
    */
    virtual void extractCondensed(int&, char**, int) {}

    /**
    * Only condensed bool are of type '-asd'
    */
    void searchCondensed(int& argc, char* argv[], const std::string& opt)
    {
        if(opt.empty()) return;

        for(int index=0; index<argc; ++index)
        {
            const size_t len = ::strlen(argv[index]);

            if(len < 2) continue;
            if( argv[index][0] != '-' ) continue;
            if( argv[index][1] == '-' ) return;

            for(uint i=1; i<len; ++i)
            {
                if( argv[index][i] == opt[0] )
                {
                    Mixin::m_value = true;
                    Mixin::m_extracted = true;

                    // remove the opt from the string.
                    argv[index][i] = argv[index][len-1];
                    argv[index][len-1] = 0;

                    break;
                }
            }
        }
    }
};

Usage

a.out --verbose -n 2 input1 input2

bodhi::CliArg< bool > isVerbose(argc, argv, "v", "verbose");
bodhi::CliArg< int > count(argc, argv, "n", "--number");

if( verbose == true ) // be verbose

argc is reduced by 2 and input1 is the next in argv

Update:

fyi I reworked my code to adhere all points made here: https://gist.github.com/aggsol/6008239

Answer 1

I agree with everything Edward said in his answer. One thing he mentions is that you have a tedious syntax that causes you to repeat some things, like the argc, argv arguments passed to the CliArg<> constructors. A potential solution is to create a generic argument parser object that you give the argc and argv parameters, and then reference that object in some way when creating your arguments.

I personally think Python's argparse module handles this in a nice way. It is not be possible to reimplement this perfectly in C++, but we can try to emulate it somehow. For example, the following usage syntax might be implementable:

parser = CliArgParser(argc, argv);
auto isVerbose = parser.addArg<bool>("v", "verbose");
auto filterName = parser.addArg<std::string>("f", "filename");
...

A very simple implementation of CliArgParser could look like this:

class CliArgParser {
    int &argc;
    char *(&argv)[];

public:
    CliArgParser(int argc, char *(&argv)[]): argc(argc), argv(argv) {}

    template<typename T>
    CliArg<T> addArg(const std::string& opt, const std::string& longOpt, const std::string& d = "") {
        return CliArg(argc, argv, opt, longopt, d);
    }
};

This requires a move constructor to be implemented for CliArg though. But once you have that, you can make it more intelligent. For example, you could keep track of which options were added to the parser, and add a function to CliArgParser() that can check whether the remaining arguments contain unknown arguments. Or you could add a help text to each argument, and have CliArgParser() print a list of all supported options along with the help text.

Answer 2

This is nice, neat code in my view, and there is absolutely nothing wrong with modifying argc and argv. Indeed, it's an entirely rational and natural way to do things for just this kind of purpose. Here are some things that may help you further improve your code:

Provide complete code to reviewers

The code as presented is not quite complete. It is missing these necessary includes and does not have a main to exercise the functions.

#include <iostream>
#include <cassert>
#include <cstring>
#include <stdexcept>
#include <string>
#include <sstream>

I moved everything to a file args.h and wrote this to test:

#include "args.h"
#include <iostream>
#include <iomanip>

#define SHOW(X) std::cout << std::boolalpha << # X " = " << (X) << '\n'

int main(int argc, char *argv[]) {
    CliArg<bool> isVerbose(argc, argv, "v", "verbose");
    CliArg<std::string> inFileName(argc, argv, "f", "filename");
    CliArg<int> count(argc, argv, "n", "number");

    SHOW(isVerbose);
    SHOW(inFileName);
    SHOW(count);

    std::cout << "Remaining args:\n";
    for (int i{0}; i < argc; ++i) {
        std::cout << "argv[" << i << "] = " << argv[i] << '\n';
    }
}

With that added, it was easier to review the code.

Provide an operator for legacy C functions

The std::string overload is very likely to be used for things such as file names that might be passed to legacy C functions, so to make that more convenient, I'd suggest adding one more function:

operator const char *() const  { return Mixin::m_value.c_str(); }

Consider repeated arguments

It's common for repeated arguments to have a defined behavior, such as "only the last one has effect" or "only the first one has effect." This code uses, in effect, the latter policy but then does not ignore or process any subsequent repeats of that option, leaving the user of this code to clean that up. It would be nice if the code handled that.

Fix the bug

The handling of combined flags, such as '-vf' in the sample code above seems to work, but has a flaw having to do with unknown or repeated arguments. If we run the code as:

src/args -n 42 -vvf config.log foo bar biz baz

The rather peculiar effect is this:

isVerbose = true
inFileName = v
count = 42
Remaining args:
argv[0] = src/args
argv[1] = config.log
argv[2] = foo
argv[3] = bar
argv[4] = biz
argv[5] = baz

The second v is misinterpreted as the inFileName while the actual file name is unprocessed.

Think of the user

With a long list of arguments, it would be rather tedious to use this syntax repeatedly:

CliArg<bool> isVerbose(argc, argv, "v", "verbose");
CliArg<bool> isCompressed(argc, argv, "c", "compressed");
CliArg<std::string> filterName(argc, argv, "f", "filename");
CliArg<std::string> waveletType(argc, argv, "w", "wavelet");
CliArg<int> count(argc, argv, "n", "number");

It might be nice to allow for a less verbose syntax or one that would support initialization of multiple variables, possibly with default values, via a constexpr data structure.

Check your spelling

In one of the comments, there's a typo: "condesed". Since this is generally nice code, it would be worth the extra effort to make sure the comments don't have spelling errors. That's the only spelling error I noticed.