Replace part of string in C++

Started by
46 comments, last by Servant of the Lord 12 years, 1 month ago
Ok, so I'm making this text adventure game so i can get into the logic of basic programming and then move on to 2D probably.

I have a misfortune though. At some point in the game the player can enter his name. So, from that point on I want the texts to have the player's name.

The thing is, to have a much simpler code and more freedom in creating the game, I read the texts from external texts files. That means I read each line of the file and then print it on the screen.

So what I thought is create a sequence of characters that I can read and replace them in the program.

So I came up with the "/NAME/" sequence. So, the player enters his name, "Nick" for example. The program should replace the "/NAME/" sequence with "Nick" and present the rest of the text normally.


So I wrote this code:

#include <iostream>
#include <cstdio>
#include <cstdlib>
#include <string>
#include <fstream>
#include <sstream>
using namespace std;

int main (){

string line;
char name[20];
fstream file;
file.open("Rebels_after_start.txt");

cin >> name;

while(file.good()){

getline(file,line);

line.replace(line.find("/N"),line.find("E/"), name ,6);
//line.replace(19,1,1,'s'); Even with this line it crashes
cout << line << '\n';
}

file.close();

}


But the program keeps getting an out_of_range instance from the replace function. I've tried all iterations of string.replace and nothing works. Even if I replace a character with a character the program crashes.

Any help please?
Advertisement
Are you sure that the string contains "/NAME/" ?
If not then you are trying it replace something that does not exists... meaning you change something outside the memory bounds of string and cause a random crash in your program.
The exact contents of the file are the following:


So, you're name is /NAME/.

The woman with the long red hair was checking him continuously. She probably was a Rebel.[/quote]

The output I get every time is this:

Nick
So, you're name is Nick
terminate called after throwing an instance of 'std::out_of_range'
what(): basic_string::replace
Aborted


------------------
(program exited with code: 134)
Press return to continue

[/quote]

What it should give me is this:

So, you're name is Nick.

The woman with the long red hair was checking him continuously. She probably was a Rebel.[/quote]

Also, as I have commented on the code even if I replace any character, with any other character the program crashes.

NOTE: I am developing under Linux Mint 12.Is there any chance this is a memory reallocation problem?
When processing the file, you read it line by line.
You get the first line. It contains /NAME/ and your code correctly replaces it and gives the desired output.
Now the next line is read. From what you posted it is an empty line. So your call to
line.find("/N")
will return string::npos. Same for
line.find("E/")
which is then passed to replace and causes the access out of bounds. Check the return values of your find calls and do not call replace if one of them is equal to string::npos.
Why dont you store the string you want to display with a %s in it. And then when it comes time to show it you sprintf the name into the buffer you use for display?

Worked on titles: CMR:DiRT2, DiRT 3, DiRT: Showdown, GRID 2, theHunter, theHunter: Primal, Mad Max, Watch Dogs: Legion

That was it! Just added

if (line.find("/N") != string::npos) line.replace( line.find("/N"),name.length()-1, name);

and it works flawlessly.


Just a question because i looked up iterators a little. An iterator is actually a reference to an element of a sequence?
Why dont you store the string you want to display with a %s in it. And then when it comes time to show it you sprintf the name into the buffer you use for display?


That would be cool too. One of those cases where in C it's actually more simple. Just to completely understand what you are talking about instead of putting "/NAME/" as a "flag" I just put %s in the text file and with sprintf i get acces to the actual buffer as if used printf(%s, name) right?

Just to completely understand what you are talking about instead of putting "/NAME/" as a "flag" I just put %s in the text file and with sprintf i get acces to the actual buffer as if used printf(%s, name) right?
[/quote]

I would recommend against using format strings loaded from external locations with sprintf(). This is a really easy way to corrupt memory, if you change either the format strings or the arguments and forget to change the other, or if your data file is corrupt. You should either write a type safe wrapper around it, which validates that the format string and arguments match one another.

Not to mention that sprintf() has a fixed buffer length.

If you were going to do this, I'd recommend keeping your current format, but switch to format strings in-memory, once you've validated that the text only contains the format strings you want. This also is good for the user, who doesn't need to worry about the types involved.


[color=#282828][font=helvetica, arial, verdana, tahoma, sans-serif]

That was it! Just added

[/font]


[color=#000088]if[color=#000000] [color=#666600]([color=#000000]line[color=#666600].[color=#000000]find[color=#666600]([color=#008800]"/N"[color=#666600])[color=#000000] [color=#666600]!=[color=#000000] [color=#000088]string[color=#666600]::[color=#000000]npos[color=#666600])[color=#000000] line[color=#666600].[color=#000000]replace[color=#666600]([color=#000000] line[color=#666600].[color=#000000]find[color=#666600]([color=#008800]"/N"[color=#666600]),[color=#000000]name[color=#666600].[color=#000000]length[color=#666600]()-[color=#006666]1[color=#666600],[color=#000000] name[color=#666600]);

[color=#282828][font=helvetica, arial, verdana, tahoma, sans-serif]

and it works flawlessly.

[/font]
[/quote]
Why not:


#include <string>
#include <iostream>

namespace {
static const std::string NAME_PLACEHOLDER = "/NAME/";
}

void format(std::string &text, const std::string &name)
{
std::string::size_type index = text.find(NAME_PLACEHOLDER);
if (index != std::string::npos) {
text.replace(index, NAME_PLACEHOLDER.length(), name);
}
}

void test(std::string text, const std::string &name) {
format(text, name);
std::cout << text << '\n';
}

int main()
{
std::string text = "So, you're name is /NAME/.\nThe woman with the long red hair was checking him continuously. She probably was a Rebel.\n...\n";

test(text, "Alexander");
std::cout << "-----------------------\n";
test(text, "Al");
}


Note that I tried your original code, and the output this the following:


So, you're name is AlexanderThe woman with the long red hair was checking him continuously. She probably was a Rebel.
...

-----------------------
So, you're name is AlNAME/.
The woman with the long red hair was checking him continuously. She probably was a Rebel.
...

[/quote]
You should test edge cases, in this situation such cases include smaller than the placeholder, and larger than the placeholder.


Just a question because i looked up iterators a little. An iterator is actually a reference to an element of a sequence?
[/quote]
Yes. It is an abstraction of the pointer/index based way of accessing elements of a sequence.


I would recommend against using format strings loaded from external locations with sprintf(). This is a really easy way to corrupt memory, if you change either the format strings or the arguments and forget to change the other, or if your data file is corrupt. You should either write a type safe wrapper around it, which validates that the format string and arguments match one another.

Not to mention that sprintf() has a fixed buffer length.


I think the latter is the more pressing concern, and it's easily remedied by using snprintf() instead (part of the C99 and C++11 standards, and most likely provided by your compiler even if you are still using C89 or C++98).

You may also want to take a look at the Boost Format library.

That was it! Just added

if (line.find("/N") != string::npos) line.replace( line.find("/N"),name.length()-1, name);

and it works flawlessly.


That's something you should wrap in a function, for future ease-of-use. smile.png

Is it intentional that you aren't searching for "/NAME/" but searching for anything that begins with "/N" and ends with "E/"? For example, "/NEW GAME/", you want to have replaced with the player's name?

Also, rhetorical question: What happens if a single line uses "/NAME/" twice? wink.png

You might want to make your own function like this:

std::string ReplaceAll(const std::string &text, const std::string &toReplace, const std::string &replaceWith)
{
std::string newText = text;

size_t pos = newText.find(toReplace);
while(pos != std::string::npos)
{
newText.replace(pos, toReplace.size(), replaceWith);
//Just to make sure we don't go into an infinite loop, if 'replaceWith' contains a match of 'toReplace'.
pos += replaceWith.size();
pos = newText.find(toReplace, pos);
}

return newText;
}


Then you can just go like this, to your heart's content:
std::string text = "Hello /NAME/, you're looking mighty spry for a /CLASS/ of /AGE/ years.\n"
"I see you are still dying your hair /HAIR-COLOR/, which is mightily becoming of a /RACE/ like yourself.\n"
"You're putting on a few pounds, however. You look like you weigh about /WEIGHT/ lbs,\n"
"and your belly is bulging out quite alot. Is that a size /PANT-SIZE/ pantaloons you are wearing?\n"
"You need to get into shape before your journey, /NAME/, if you don't mind me pointing out the obvious.\n";

text = ReplaceAll(text, "/NAME/", playerName);
text = ReplaceAll(text, "/AGE/", playerAge);
text = ReplaceAll(text, "/WEIGHT/", playerAge);
text = ReplaceAll(text, "/CLASS/", playerClass);
text = ReplaceAll(text, "/RACE/", playerRace);
text = ReplaceAll(text, "/HAIR-COLOR/", playerHairColor);
text = ReplaceAll(text, "/PANT-SIZE/", playerPantSize);

std::cout << text << std::endl;

This topic is closed to new replies.

Advertisement