[C++] String Manipulation

Started by
7 comments, last by ToohrVyk 16 years, 2 months ago
First of all, here is the exercise (it was given to me on this forum): "Mad Libs. Read an input file that contains placeholders marked by square brackets (you may assume there are no other instances of square brackets in the file). As you read not-enclosed text, append it to an output string. When you find a square bracket, read the text between the square brackets (it will be some kind of description, like "proper noun") and prompt the user to supply something of that sort. "Fill in the blank" by appending it to the output string. Once you reach the end of the file, output the whole output string." I'm running into a problem. I cannot seem to edit strings because I made them into arrays. I understand that even if I got this working, it wouldn't validly replace the words between []. I'm just seeing how I can replace string characters. Anyways, here is the code:
#include "stdafx.h"
#include <iostream>
#include <string>
#include <sstream>
#include <fstream>

using namespace std;

int _tmain(int argc, _TCHAR* argv[])
{
  int i = 0;
  int x = 0;
  string sFileline;
  string sUserline[100];
  string sSaveline[100];
  ifstream libfile ("madlibs.txt");
  if (libfile.is_open()){
		while (! libfile.eof()){
			getline (libfile, sFileline);
			cout << sFileline << endl;
			sSaveline[x] = sFileline;
			++x;
			if (sFileline.find('[')){
				cin >> sUserline;
				++i;
			}
  }
    libfile.close();
  }
  else
	  cout << "The file cannot be opened!";
  //THE BELOW FOR LOOP IS WHERE THE PROBLEM LIES!
  //I GET THIS ERROR: "Unhandled exception at 0x7c812a5b in Mad Libs.exe: Microsoft C++ exception: std::out_of_range at memory location 0x0012e3ac.."
  for (int i = 0; i < sizeof(sUserline); ++i){	
		  sSaveline.capacity();
		  string replacement = sUserline;
		  string sought = "[";
		  sSaveline.replace(sSaveline.find(sought), sought.size(), replacement); 
  }
  system("PAUSE");
  return 0;
}
I know that the array is causing the problem. I know this because I was able to do this in a separate test without the array. This is the test code worked fine:
#include "stdafx.h"
#include <iostream>
#include <string>
#include <sstream>

using namespace std;

int _tmain(int argc, _TCHAR* argv[])
{
string phrase = "This is a test";

cout<< "original string capacity: " <<phrase.capacity()<<endl;
string replacement = "test to check if the array is causing problems!";
string sought = "test";
phrase.replace(phrase.find(sought), sought.size(), replacement);
cout << phrase << endl;
cout << "new capacity: "<< phrase.capacity() << endl;

system("PAUSE");
}
TL;DR;, How can I properly manipulate a string's characters and length if the string is in an array? Thank you in advance!
Advertisement
for (int i = 0; i < sizeof(sUserline); ++i)
That line loops from 0 to the size in bytes of the memory occupied by the string object. You probably want:
for (int i = 0; i < sUserline.length(); ++i)

EDIT: I'm talking bollocks... What exactly are you trying to do here? Do you want to loop over all strings in the array of 100, or all characters in one string?
A few things I noticed:

1. I don't think getting the file line by line is the best way to go here. The way you have it, you're going to keep spitting out lines you read from the file. Then when you find a line that has a word enclosed in brackets, you're asking the user to put in that entire line. Instead, why not grab the text one word at a time? Like this:

while ( !libfile.eof() ){     string sCurrentWord;     libfile >> sCurrentWord;     // do other stuff}


When you use the ">>" operator on a string like that, the ifstream will automatically look for spaces and line breaks so that you just get a single word. I think that would be useful in your case.


2. There's really no need for you to use arrays at all here. In fact, the way you're using them is basically just guaranteeing a buffer overflow in certain cases. Think about it: your array of strings has space for 100 lines. What happens if your text file has 101 lines? Since you're not checking for this case, you'll write to the non-existent 101th array index and end up corrupting some memory. Instead, I suggest using the string's "+" operator to perform string concatenation. See when you do this:

string string1 = "I ";string string2 = "Rule!";string1 += string2;cout << string1;


You'd get the output "I Rule!" on the console. This means that as you're pulling strings out of your input file, you can simply keep adding the new string to a single string containing all of the text you've read so far. You can also do this as much as you want and you don't have to worry about running out of space like you do with an array, since string will keep increasing the size of its internal array as needed. If you really want to use arrays, I would suggest looking up the std::vector class which behaves like an array but will resize itself as you add new elements.

3. As Evil Steve said, sizeof( sUserline ) does not give you the length of a string. sizeof returns the amount of bytes in memory a type uses, and for complex reasons this is unrelated to how long the actual string of characters is. Instead, just use the length() method the string class to find out how long it is. Like this:

for (int i = 0; i < sUserline.length(); ++i)


Quote:Original post by Evil Steve
for (int i = 0; i < sizeof(sUserline); ++i)
That line loops from 0 to the size in bytes of the memory occupied by the string object. You probably want:
for (int i = 0; i < sUserline.length(); ++i)

EDIT: I'm talking bollocks... What exactly are you trying to do here? Do you want to loop over all strings in the array of 100, or all characters in one string?


I want to loop over all strings in the array of 100. I understand that my looping isn't very good, and I would rather make an array based on the number of lines in the file that I load from, but I'll learn that later.

I'm fairly certain that the problem lies in editing the string's length because it is an array. The second code segment I wrote worked, I THINK, because I wasn't manipulating a string array. I could be wrong though, as a newb I'm not very confident in these statements!

I am, or at least I hope I am, getting lines from a file. Then I'm making a string array, called sSaveline, where each element contains each line. Then I'm checking for the character "[" in each line. For each "[", I allow user input to replace the text between the [] (I understand that this doesn't do that yet, I just want to know how to replace the "[" at this point before I worry about that). The user's input for each line is loaded into an array called sUsersline. Then, for the for-loop, I run through the number of elements in the sUsersline (which is the same amount as the sSaveline). Then I am replacing the "[" in each sSaveline element with the corresponding sUsersline element.
Note:
while ( !libfile.eof() ){     string sCurrentWord;     libfile >> sCurrentWord;     // do other stuff}


This won't work. The Standard C++ Library streams only set the end of file bit *after* a failed read. The idiomatic way would be to use the read operation itself as the condition (which also handles non-eof errors nicely):
// word by wordstring word;while ( file >> word ){    // other stuff}// line by linestring line;while ( getline(file,line) ){    // other stuff}
Quote:Original post by MJP
A few things I noticed:

1. I don't think getting the file line by line is the best way to go here. The way you have it, you're going to keep spitting out lines you read from the file. Then when you find a line that has a word enclosed in brackets, you're asking the user to put in that entire line. Instead, why not grab the text one word at a time? Like this:

while ( !libfile.eof() ){     string sCurrentWord;     libfile >> sCurrentWord;     // do other stuff}


When you use the ">>" operator on a string like that, the ifstream will automatically look for spaces and line breaks so that you just get a single word. I think that would be useful in your case.


2. There's really no need for you to use arrays at all here. In fact, the way you're using them is basically just guaranteeing a buffer overflow in certain cases. Think about it: your array of strings has space for 100 lines. What happens if your text file has 101 lines? Since you're not checking for this case, you'll write to the non-existent 101th array index and end up corrupting some memory. Instead, I suggest using the string's "+" operator to perform string concatenation. See when you do this:

string string1 = "I ";string string2 = "Rule!";string1 += string2;cout << string1;


You'd get the output "I Rule!" on the console. This means that as you're pulling strings out of your input file, you can simply keep adding the new string to a single string containing all of the text you've read so far. You can also do this as much as you want and you don't have to worry about running out of space like you do with an array, since string will keep increasing the size of its internal array as needed. If you really want to use arrays, I would suggest looking up the std::vector class which behaves like an array but will resize itself as you add new elements.

3. As Evil Steve said, sizeof( sUserline ) does not give you the length of a string. sizeof returns the amount of bytes in memory a type uses, and for complex reasons this is unrelated to how long the actual string of characters is. Instead, just use the length() method the string class to find out how long it is. Like this:

for (int i = 0; i < sUserline.length(); ++i)


OK, I'll look into std::vector class. As for the other problems, such as the buffer overflow, I knew they were there. I planned on learning that after I fixed the string manipulation problem, but thanks for helping me with them before I looked them up. I might ask some more questions once I work on this some more... Thanks again!
Quote:Original post by rippingcorpse

OK, I'll look into std::vector class. As for the other problems, such as the buffer overflow, I knew they were there. I planned on learning that after I fixed the string manipulation problem, but thanks for helping me with them before I looked them up. I might ask some more questions once I work on this some more... Thanks again!


Ah okay, that's good you knew about the buffer overflow. I usually just explain everything I can, just in case someone doesn't know.

As for std::vector, it's very easy to use. Here's a simple example:
#include <vector>vector<int> intVector;for ( int i = 0; i < 10; i++ )     intVector.push_back( i );cout << intVector[5] << endl;     // Outputs "5"for ( int i = 0; i < intVector.size(); i++ )     cout << intVector;        // Outputs "0123456789"


So basically push_back adds a new element to the end of the vector, and size tells you how many elements are in it. Then you can access elements like an array, using the brackets (just make sure you access an index that's < size).

I tried this and it passes as true for every word even though not every word contains "]". Does anyone know why?

if (sCurrentWord.find(']')){	    cin >> sUserline;    sUserline = sCurrentWord;}
Quote:Original post by rippingcorpse
I tried this and it passes as true for every word even though not every word contains "]". Does anyone know why?


std::string::find. Note the return type.

This topic is closed to new replies.

Advertisement