Inserting strings in middle of a text file using streams

Question

I have a text file in a specific format that I pass as the input of my program. I then want to write the output back to that same file. The format is as follows:

Input1:
1 2 3 4 5 6 7

1 2 3 4 5 6 7

1 2 3 4 5 6 7
-----------------------------------------
Input2:
1 2 3 8 5 6 7

1 2 3 9 5 6 7

1 2 3 10 5 6 7
-----------------------------------------
Input3:
1 2 3 4 5 6 7

1 2 3 4 5 6 7

1 2 3 4 5 6 7

If I want to process Input2 and add the output, the resulting file should be:

Input1:
1 2 3 4 5 6 7

1 2 3 4 5 6 7

1 2 3 4 5 6 7
-----------------------------------------
Input2:
1 2 3 8 5 6 7

1 2 3 9 5 6 7

1 2 3 10 5 6 7
    output: 1 2 3 4 5 6 7
-----------------------------------------
Input3:
1 2 3 4 5 6 7

1 2 3 4 5 6 7

1 2 3 4 5 6 7

I am looking for a way to insert lines into that section. For that I wrote the following routine:

//string fileName: Filename on which read and write operation are being done
//int outputNumber: Number of output to be written
//int nInputLine: How many lines each input is taking
//int nOuputLine: How many lines each output is taking. For now it is not     
//being used
//string outpur: Output to be Written
void CFirstIO::WriteOutPut(string fileName, int outPutNumber, int  
 nInputLine, int nOutPutLine, string outPut)
{
  ifstream fileSt(fileName.c_str(),ios::binary);
      if(fileSt.good() && fileSt.is_open())
  {
    //Storing the content of the file in a string
    fileSt.seekg(0,ios::end);
    int nLength=fileSt.tellg();
    fileSt.seekg(0,ios::beg);
    char *buff = new char[nLength];
    fileSt.read(buff,nLength);
    string strFileContent(buff); 
    delete []buff;

    string strToFind;
    char formatinput[256];
    sprintf(formatinput,"Input%d:",outPutNumber);
    strToFind+=formatinput;

    fileSt.seekg(0,ios::beg);
    int nPos= strFileContent.find(strToFind,0);
    fileSt.seekg(nPos);      

    //Trying to reach at the bytes Number where I want to insert the 
    //output
    string sLine;
    for(int i=0; i<nInputLine+1;i++)
    {
        if(!fileSt.eof() && fileSt.good())
        getline(fileSt,sLine);
    }

    nPos=fileSt.tellg();
    strFileContent.insert(nPos,outPut);
    fileSt.close();

    ofstream fileOut(fileName.c_str(),ios::out|ios::trunc);
    fileOut<<strFileContent;

    fileOut.close();      


    }
}

This function works, but adds some unintended characters at the end of the file when reading the whole file in the buffer.

All in all, this function does not look nice to me as I feel like I'm not using the full potential of the standard library. How could my code be improved?

I was thinking of getting it reviewed but my solution was so bad that it became diagnostic problem..
Then it does not belong on codereview.stackexchange.com; it doesn't really belong on stackoverflow.com either, but plenty of diagnostic problems seem to end up there anyway

Anton Golov · Accepted Answer · 2012-02-25 20:41:04Z

First and foremost, fix your formatting. A little bit of effort spent on making the code readable will mean that it can be understood (and thus reviewed) faster. By the way, you are not using the STL.

Now, significant issues in order of when they appear in your code:

Use an std::vector in favour of new[] pretty much always. In this case, as the content is all text, an std::string may be even better, but causes problems due to you wanting to read directly into it.
As you want to work with the input on a line-by-line basis, I would advise an std::vector<std::string> that you can populate using a std::string line; while (fileSt >> line) lines.push_back(line);. There may be more efficient solutions, but check that you really need them first.
What you're doing with sprintf doesn't make sense. That could all be replaced with std::string strToFind = "Input" + boost::lexical_cast<std::string>(outPutNumber);.
Notice that if you store the strings as I recommended above, finding a certain line is as simple as lines[nInputLine].
To write everything back, just insert another line into the vector and then write it using a loop or with stream iterators.
No need to explicitly close the files unless you care about the exception that may throw.

And some minor issues:

Your comments are inconsistent, and duplicate information. You have specified the type everywhere except for in filename, despite the type already being given by the signature.
No need to check for fileSt.good() when checking if its open.
As your function only does anything if the file is opened successfully, return early instead of wrapping the whole thing in a giant if.
Be consistent with qualifying things with std::; do it or don't do it, but don't mix it. (I would advise explicitly qualifying things above using-declarations (and even more so above using-directives), but all three may be acceptable.)
Same with Hungarian notation: even if you don't agree that the qualifiers don't help with anything, you should at least either use them consistently.

Manish · Answer 2 · 2012-02-28 16:07:13Z

Srry for replying so late to your comments and thanks for your suggestions..With the help of your inputs I could come up with the easy and much cleaner solution for my problem.. Here is my updated code.

//string fileName: Filename on which read and write operation are being done
//int outputNumber: Number of output to be written
//int nInputLine: How many lines each input is taking
//int nOuputLine: How many lines each output is taking. For now it is not being used
//string output: Output to be Written
void CFirstIO::WriteOutPut(string fileName, int outPutNumber, int nInputLine, 
                           int nOutPutLine, string outPut)
{    
    ifstream fileSt(fileName.c_str(),ios::in);
    if(fileSt.is_open())
    {
      vector<string>lines;
      string line;
      while(getline(fileSt,line))
     {
        lines.push_back(line);
     }
     fileSt.close();
     stringstream ss;
     ss<<"Input"<<outPutNumber<<":";
     string toFind=ss.str();
     vector<string>::iterator it=find(lines.begin(),lines.end(),toFind);
     lines.insert(it+nInputLine+1,outPut);
     fstream outSt(fileName.c_str(),ios::out);
     if(outSt.is_open())
    {
        for(vector<string>::iterator it=lines.begin();it!=lines.end();it++)
        {
            outSt<<*it<<"\n";
        }

        outSt.close();  
    }
   }
}

Now the only problem I see with this code is absence of error handling..And as I am not using the boost library I used string stream for converting the 'int' to the string. And now I think I can say I am using STL..:P

You should edit the code into the question (separately from what you already have), and then make a comment. You're reading the stream wrong (you'll read one line past the end). You're still explicitly closing streams (although fileSt failing to close wouldn't stop you from writing outSt). As I said, you are (fortunately) not using the STL.
@AntonGolov:Could not understand past the end..If you are talking about "while(!fileSt.eof())" than as far as I understand it is correct. For example like if lines are l1,l2,l3 and I start reading from l1,after l3 it will hit eof and so I will be able to read all lines..Correct me if I am wrong.And about STL part as I read the article on the link provided by you, I got to know that string,stream are not part of the STL but C++ stdlib but here I am also using vector and iterator so isn't it the usage of STL..
No, you should be checking the return value of std::getline, fileSt.eof() will not tell you anything. See latedev.wordpress.com/2011/09/13/… . And no, see kera.name/articles/2010/08/it-is-not-called-the-stl-mmkay .

asked	1 year ago
viewed	1178 times
active	1 year ago

Inserting strings in middle of a text file using streams

2 Answers

Your Answer

Not the answer you're looking for? Browse other questions tagged c++ or ask your own question.

Inserting strings in middle of a text file using streams

2 Answers

Your Answer

Sign up or log in

Post as a guest

Not the answer you're looking for? Browse other questions tagged c++ or ask your own question.

Related