string::begin and string::replace - bad combination?

[Zapeth]

Hi so I want to go through a whole string and replace all occurrences of a newline with a ", ".
I am doing this by iterating through the string (so I know when I'm done) and in the loop I use the search method to look for the next newline occurrence and then use replace to put a ", " into the string instead.

That works for the first 5 runs, however in the 6th run, the iterator suddenly contains invalid data after the replace method is called and the program crashes in the next runthrough with the error message

Debug Assertion Failed! ... Expression: string iterator not incrementable

Here is my code:

string formatString( char* unformatted )
{
	int pos = 0;
	string buffer = unformatted;
	string::iterator it;

	for ( it = buffer.begin(); it != buffer.end(); it++)
	{
		pos = buffer.find('\n', pos);
		buffer.replace(pos, 1, ", ");	// in the sixth run 'it' is "deleted"
	}

	return buffer;
}

Is this code correct or am I missing something here?

[FlashDrive]

AFAIK, if the error message says that a string iterator is not incrementable, you are not allowed to make something like you did in line 7:
it++
But I don't have another solution... Sorry. =(

[Zapeth]

Then it wouldn't have worked the first 5 times.

Also its pretty much exactly coded like this in the reference here (at least the loop structure).

[FlashDrive]

But why else would the program say:

Debug Assertion Failed! ... Expression: string iterator not incrementable

[Zapeth]

Because as I said the iterator suddenly contains no valid data, it's like the memory was free'd.

But maybe you have a different suggestion of how to solve this problem? (to format a string that contains newlines and replace them with ", ")

[Disch]

strings are basically glorified vectors. In a vector, when you resize, the vector might have to copy the contents to a larger buffer, which invalidates all iterators.

Same is true of a string. When you modify a string, it's possible that the string will need to move the data to another buffer in memory. If it does this, all your iterators become garbage because they're pointing to data that has been moved and no longer exists.

This is not a safe thing to do. The fact that this worked the first 5 times is a fluke. You were just getting "lucky" that the string data hadn't been moved in memory.


I don't see the point of that for loop anyway. Why do you have a for loop spinning on an iterator when you aren't even using the iterator inside the loop?

EDIT:

If the goal is to replace all occurances of "\n" with ", ", then this should work:

size_t pos = 0;
while(true)
{
  pos = buffer.find( '\n', pos );
  if(pos == string::npos)  // no more occurances found
    break;

  buffer.replace( pos, 1, ", " );
}

[naraku9333]

Try a while loop instead

while((pos = buffer.find('\n',pos)) != -1){
		buffer.replace(pos, 1, ", ");
	}

[Zapeth]

lol you are right, for some reason I thought I needed an iterator (stupid brain).
though it's interesting to know this.

So I guess I need to count the number of newlines before remvoing them so I know when i'm done or is there an easier way?

I could think of a while loop instead of searching for all occurences and then again going through the string and replacing them.

But I can't think of a way to use the search method only once or is there a way? If yes please post an example ;)

[ciphermagi]

They already did.

[Galik]

The for-while version...

for(size_t p = 0; (p = buffer.find('\n', p)) != std::string::npos; p += 2)
		buffer.replace(p, 1, ", ");

[Zapeth]

oh sry I didn't see that, I could have sworn that it wasn't there when I sent my post.

Anyway, the loop is legit like this?
while((pos = buffer.find('\n',pos)) != -1)
I would have thought that the condition here would be like "Try to set pos then compare the result of the setting to -1" so it would be an infinite loop because the result could only be a boolean which is 0 or 1 but never -1.

I'll try it out and see if it works though, thanks for all the answers.

edit: works of course^^
and thank you too for the for-while example Galik ;)

[Disch]

if((x = y) != -1)

is the same as

x = y;
if(x != -1)

naraku's code is just compacted into fewer lines, but it essentially does the same thing as my example.

while((pos = buffer.find('\n',pos)) != -1) <- sets 'pos' to the result of find(). Then checks to see if pos is -1. If it isn't, the loop continues, if it is (no more items found), the loop exits.

I could have sworn that it wasn't there when I sent my post.

It might not have been. I edited my post to add the example after my initial post. Perhaps you saw it pre-edit.

[Zapeth]

But if I have conditions like if(pos = 1)then it would return a boolean if the allocation was successful, thats why I thought the given example would end in an infinite loop.

Or perhaps there is also a way to write the expression like my above code?

[Disch]

But if I have conditions like if(pos = 1)then it would return a boolean if the allocation was successful

No.

if(pos = 1)

is the same as

pos = 1;
if(pos)

And since any nonzero value is "true", that is also the same as:

pos = 1;
if(pos != 0)

[Zapeth]

okay, thanks for the explanation ;)