"vicente.botet" <vicente.botet@wanadoo.fr> wrote in message news:06810FA68C344871BDFED63A5595AF0D@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