Re: [boost] [program_options]

Subject: Re: [boost] [program_options]
From: Domagoj Saric (domagoj.saric_at_[hidden])
Date: 2010-03-08 06:31:36

"vicente.botet" <vicente.botet_at_[hidden]> wrote in message
news:06810FA68C344871BDFED63A5595AF0D_at_viboes1...
...
> What about having both overloads? const char * is the best for literals, while
> string cont& is the best for strings variables.
>
> void option_description::set_name(string const&_name);
> void option_description::set_name(const char* _name);
...

Well, looking at the implementation of set_name() posted above:

option_description::set_name(const char* _name)
{
    std::string name(_name);
    string::size_type n = name.find(',');
    if (n != string::npos) {
        assert(n == name.size()-2);
        m_long_name = name.substr(0, n);
        m_short_name = '-' + name.substr(n+1,1);
    } else {
        m_long_name = name;
    }
    return *this;
}

it can be noticed that it, in the case where a comma is present in the
string, makes 4 to 6 allocations (and copies) when it should make 0 to 2...and
in the 'other' case makes 1 or 2 allocations when it should make 0 or 1...

If we change the implementation as follows:

typedef iterator_range<char const *> string_chunk_t;

option_description::set_name( string_chunk_t const name )
{
    string_chunk_t::const_iterator const pComma( std::find( name.begin(),
        name.end(), ',' ) );
    if ( pComma != name.end() ) {
        assert( pComma == name.end() - 2 );
        char const short_name[ 2 ] = { '-', name.back() };
        m_short_name.assign( boost::begin( short_name ), boost::end(
           short_name ) );
    }
    m_long_name.assign( name.begin(), pComma );
    return *this;
}

we solve both the inefficency issues outlined above as well as the char const *
/ std::string const & overload issues. It will even eliminate the strlen() calls
for literals.

Unfortunately, because of certain issues with boost::range (that I will go into
more detail in a separate post) the following two overloads are currently
also necessary:

template <class Range>
option_description::set_name( Range const & string_range, typename
disable_if<is_pointer<Range>>::type * = 0 )
{
    char const * const begin(   &*  boost::begin( string_range )             );
    char const * const end  ( ( &*( boost::end  ( string_range ) - 1 ) ) + 1 );
    assert( is_array<Range>::value == ( *(end - 1) == '\0' ) );
    set_name( string_chunk_t( begin, is_array<Range>::value ? end - 1 : end ) );
}

template <class Range>
option_description::set_name( Range const & name, typename
enable_if<is_pointer<Range>>::type * = 0 )
{
    set_name( boost::as_literal( name ) );
}

ps. "From the looks of that assertion" it seems the suggested implementation can
be further improved by eliminating even the std::find() operation:

option_description::set_name( string_chunk_t const name )
{
    string_chunk_t::const_iterator pComma( name.end() );
    if ( *( name.end() - 2 ) == ',' ) {
        pComma = name.end() - 2;
        char const short_name[ 2 ] = { '-', name.back() };
        m_short_name.assign( boost::begin( short_name ), boost::end(
            short_name ) );
    }
    else
        assert( std::find( name.begin(), name.end(), ',' ) == name.end() );

    m_long_name.assign( name.begin(), pComma );
    return *this;
}

Now, if this is necessary, the strong exception safety guarantee could be
provided by first calling reserve() on m_long_name at the beginning of the
function....

--
"What Huxley teaches is that in the age of advanced technology, spiritual
devastation is more likely to come from an enemy with a smiling face than from
one whose countenance exudes suspicion and hate."
Neil Postman