# C++ std::string.insert() question.

## Recommended Posts

Hi, The following code extracts elements from a string representing a binary value. It correctly exacts the elements, however, when it inserts that element to 'binaryString' it also inserts a smiley face or playing card character. It seems like the characters being inserted are in ASCII order. Black smiley, white smiley, heart, diamond, club, etc. Regardless of the values of startBit and stopBit the characters always start with the black smiley and work their way up. When I display 'result' to the console it shows only a "0" or "1" without the smiley face. When it's inserted, 'binaryString' gets the "0" or "1" along with the smiley face, card character, etc.

// BitName is an enum whose values are 0 through 7.

std::string binaryString( "" );

for ( BitName bit = startBit; bit >= stopBit; bit = BitName(bit - 1) )
{
char result( m_binary.at(bit) );
binaryString.insert(0, &result);
}


The following gives me the desired output, that is, no smiley faces. I've changed insert() to insert only one character. Why does the above code do what it does? What am I not understanding here?

std::string binaryString( "" );

for ( BitName bit = startBit; bit >= stopBit; bit = BitName(bit - 1) )
{
char result( m_binary.at(bit) );
binaryString.insert(0, &result, 1);
}



##### Share on other sites
std::basic_string<T,,>::insert(0, T *) expect the second argument to be a null terminated string. You're passing it the address of a single character, but don't give that character a terminating null.

##### Share on other sites
The overload of insert that takes a char* without a length assumes the pointer refers to a null-terminated character array. Your single character, obviously, isn't.

##### Share on other sites
You are using a wrong overload of the insert method. That is for C-strings, you want one for single characters, e.g

string.insert(0, 1, my_char);string.insert(string.begin(), my_char); //my favourite of these//or even: treat the char as a C-string, but just request one character from itstring.insert(0, &my_char, 1);

However, instead of inserting all the characters one by one to the beginning of the string, perhaps it would be more efficient and simpler to append them and then reverse the string:

string += my_char;//when donestd::reverse(string.begin(), string.end());

##### Share on other sites
Thanks, again. I always get an education when I post here.

visitor, thanks. I'll look into that. I knew there was a reverse() somewhere but in my annoyance/ignorance at not getting insert() to work I neglected to go find it. It definitely offers some possibilities because just above that snippet of code is another loop that looks very similar except it wouldn't require a reversal of the string because the user entered start and stop in a different order.

##### Share on other sites
Quote:
 Original post by BelgiumThanks, again. I always get an education when I post here.visitor, thanks. I'll look into that. I knew there was a reverse() somewhere but in my annoyance/ignorance at not getting insert() to work I neglected to go find it. It definitely offers some possibilities because just above that snippet of code is another loop that looks very similar except it wouldn't require a reversal of the string because the user entered start and stop in a different order.

Um... what exactly is the full set of code supposed to do? I suspect you're over-thinking things. Show the entire function and I'll see what ideas I can give you.

##### Share on other sites
Quote:
 Original post by ZahlmanUm... what exactly is the full set of code supposed to do? I suspect you're over-thinking things. Show the entire function and I'll see what ideas I can give you.

The function has changed a bit since I last posted and I probably am overthinking it.

The function's task was to retrieve user specified bits from a string representing the eight bits of an octet. Initially, the user was to specify the start and stop bits with the most-significant-bit being the first argument and the least-significant-bit being the second argument. Then I started thinking, what do I do if the user puts the arguments in the other way around? I decided just to accept it and return the correct bits as if the user put them in correctly. That's where the loop came from which prompted this post.

However, now there is only one loop.

Anyways, here's what I ended up with. Critique is welcome.

std::string KlOctet::getBinary(const BitName& start, const BitName& stop) const{    // Remember, BitName starts with B7 and descends to B0.  However, internally, B7 == 0    // and B0 == 7.  This may make some of the following look weird.    // Ensure that 'start' is the most-significant-bit and 'stop' is the least-significant-bit.    // If not, swap them so this is true.     BitName startBit( start );    BitName stopBit( stop );    if ( startBit > stopBit )    {        std::swap(startBit, stopBit);    }    // Extract each bit and add it to binaryString in the order they were extracted.  This will    // result in the most-significant-bit being the leftmost element in the string and the    // least-significant bit being the rightmost element in the string.		    std::string binaryString( "" );       for ( BitName bit = startBit; bit <= stopBit || bit == startBit; bit = BitName(bit + 1) )    {        char result( m_binary.at(bit) );         binaryString += std::string(&result, 1);    }    return binaryString;}

##### Share on other sites
Instead of passing by const-reference and then making your own copies, just pass by value. That way, you make the copies that you need anyway, and you don't have to come up with separate names for the parameters and the copies.

'bit == start' can't ever happen in your for-loop, unless 'BitName()' does some kind of looping logic. I was assuming BitName is just an enumeration, though, such that this is just a cast.

Consider overloading operator++ for BitName. That way you can make it do the looping logic if necessary, too.

Oh, and you can append a single char to a std::string with operator+ and it does what you want; no need to mess around creating a string. (You can append a null-terminated char*, too.) (And what's in m_binary that you have to cast the result to char? If it's a vector of char, then no cast is needed; if it's a vector of bool, then the cast will give you 0 and 1 bytes rather than '0' and '1' symbols.)

// I prefer 'asBinary' as a name because it implies an alternate representation// for the data, rather than simple access.std::string KlOctet::asBinary(BitName start, BitName stop) const {    if (start > stop) {        std::swap(start, stop);    }    std::string binaryString;       for (BitName bit = start; bit <= stop || bit == start; ++bit) {        binaryString += char(m_binary.at(bit));    }    return binaryString;}

##### Share on other sites
Quote:
 Original post by ZahlmanInstead of passing by const-reference and then making your own copies, just pass by value. That way, you make the copies that you need anyway, and you don't have to come up with separate names for the parameters and the copies.'bit == start' can't ever happen in your for-loop, unless 'BitName()' does some kind of looping logic. I was assuming BitName is just an enumeration, though, such that this is just a cast.Consider overloading operator++ for BitName. That way you can make it do the looping logic if necessary, too.Oh, and you can append a single char to a std::string with operator+ and it does what you want; no need to mess around creating a string. (You can append a null-terminated char*, too.) (And what's in m_binary that you have to cast the result to char? If it's a vector of char, then no cast is needed; if it's a vector of bool, then the cast will give you 0 and 1 bytes rather than '0' and '1' symbols.)

Thanks! This was great stuff for me. Thank you for introducing me to the word 'as'. It reads much better that 'get' and I'm amazed that I had never considering using it before.

Your question regarding what's in m_binary also sent me down a line of thinking and made me realize that I don't even need the loop. m_binary is a string. The stuff I need pointed out to me with huge flashing neon signs...

## Create an account

Register a new account

• ### Forum Statistics

• Total Topics
628333
• Total Posts
2982121

• 22
• 9
• 9
• 13
• 11