Appending to a string won't work twice in a row.

[ShadowTek]

I'm trying to simply append 3 strings together into one string, but I can only get append to be successful once. I thought that there may be some kind of issue with reserving space for the string, so I tried to explicitly do so, but that didn't work either.

Here is what I was trying to create, step by step.

Maps\
Maps\Abbottsburg
Maps\Abbottsburg.tif // This is the goal.
Maps\
Maps\Abbottsburg
Maps\Abbottsburg.tif // This is the goal.

I can only get this to work if I use a constant string, but I need to use a string array. When I try to refer to the element in the array that contains the first appended element (Abbottsburg in the previous case), this is what results:

Maps\
Maps\Abbottsburg
.tif\Abbottsburg
Maps\
Maps\Acme
.tif\Acme

The third string that I try to use to append for the second time ends up overwriting the first, original part of the string, yet the size() returned confirms that the string did in fact grow by 4 characters after the second appending in each case.

Here is the code I am using. There is a lot of other stuff, but the current issue involves the area marked in bold near the bottom.

#include <stdio.h>
#include <iostream>
#include <fstream>
#include <string>
using namespace std;
char * MasterList = "OutputFile.txt";
bool ErrorOccurred = false;
string FilePath = "Maps\\";
string MapFileExt = ".tif";



int main ()
{
     string * MasterListLines;
     ifstream MasterListStream (MasterList, ios::binary);

     string TempString;
     int NumberOfLines = 0;

     if (MasterListStream.is_open())
     {
                                    while (! MasterListStream.eof())
                                          {
                                                                    getline (MasterListStream, TempString);
                                                                    ++NumberOfLines;
                                                                    }
                                                                    MasterListStream.close();
                                                                    }
     else cout << "Unable to open file.";
     
     MasterListLines = new string[NumberOfLines];
       
     MasterListStream.clear();
     MasterListStream.open (MasterList, ios::binary);
     if (MasterListStream.is_open())
     {
                                    while (! MasterListStream.eof())
                                    {
                                          for (int n=1; n<=NumberOfLines; ++n)
                                          {
                                              getline (MasterListStream, MasterListLines[(n-1)]);
                                              if (! MasterListStream.good() && ! MasterListStream.eof())
                                                 ErrorOccurred = true;
                                          }
                                     }
     }
     else
         cout << "Unable to open file.";
     if (ErrorOccurred == true)
       cout << "WARNING: SOME ERROR OCCURRED!"; 
       
     MasterListStream.close();
// THIS BEGINS THE AREA OF CONCERN.
     string NewName;
     string OldName;
     int OpResult;
     for (int n=0; n<NumberOfLines; n+=4) // This loop only runs twice.
     {
         NewName = FilePath;
         OldName = FilePath;
         NewName.reserve(512);
         OldName.reserve(512);
/*
           cout << NewName.size() << endl;
           cout << NewName.capacity() << endl;
*/
           cout << NewName << endl;
         NewName.append (MasterListLines[n]); /* <------ This Is what I want to use. This is what was used in the second example above. */
//         NewName.append ("Abbottsburg");    /* <----- For some reasom, using this instead will make it work. This is what was used in the first example above. */

/* 
           cout << NewName.size() << endl;
           cout << NewName.capacity() << endl;
*/
           cout << NewName << endl;
         OldName.append (MasterListLines[n+1]);
         NewName.reserve(512);
/*
           cout << NewName.size() << endl;
           cout << NewName.capacity() << endl;
*/
         NewName.append (MapFileExt);
/*
           cout << NewName.size() << endl;
           cout << NewName.capacity() << endl;
*/
           cout << NewName << endl;
           }
    delete[] MasterListLines;
    return 0;
}

As you can see, I was thinking that space allocation might have something to do with it, but I really have no idea. I may be missing something elementary, but I can't find any helpful information in any of my online searches.

Anybody know what the problem is?

[firedraco]

I would probably check MasterListLines[] and make sure the data is actually in there...if it is, I don't really see why it wouldn't work.

[ShadowTek]

Yes, I had already confirmed that before I moved on to this part of the script. You can see what their contents are in the mangled output file I posted.

Maps\
Maps\Abbottsburg
.tif\Abbottsburg
Maps\
Maps\Acme
.tif\Acme

You can see that "Maps\\" and ".tif" are declared as globals at the very start of the script, and the contents of both retrieved elements from MasterListLines[], "Abbottsburg" and "Acme", are successfully retrieved, and even remain at the end of the last line output.

I just don't understand why using that array causes the third introduced element to start at the beginning of the string, overwriting the first characters, while using a constant instead will result in success. And there is still the strange issue of the size of the third line being reported as being +4 larger, when it looks like it's the same length as the second line. If you uncomment all those size() and capacity() reports, you'll see what I mean.

I just didn't know if there was something simple that I was missing.

If I can't figure this out, I'm just going to have to consider append() to be unreliable.

Btw, here is the file that MasterListLines[] is reading from:
OutputFile.txt

Abbottsburg
o34078e6.tif
http://www.archive.org/download/usgs_drg_nc_34078_e6/o34078e6.tif

Acme
o34078c2.tif
http://www.archive.org/download/usgs_drg_nc_34078_c2/o34078c2.tif

[mahlerfive]

It seems right... try adding a printout of MasterListLines[n], MasterListLines[n+1], MasterListLines[n+2] at the start of the inside of the loop to make sure they are what you expect.

[guestgulkan]

a couple of things:

1. remove the ios::binary because it sucks in the carriage returns at the ned of the lines and adds them to the lines/text

This may seem a bit harsh
2. The program is doing exactly what you have programmed it to do - so if the output is "wrong" then Your logic is wrong. Sorry

[mahlerfive]

To expand on what guestgulkan said, the problem is most certainly not with the append method, but it is something you have done wrong that is just very tricky to spot. I can say out of experience that every time I've spent sooooo much time trying to find a bug that I'm sure it is the compiler/IDE/library's fault, 99.9% of the time it ended up being my fault. I can only think of two cases in my life where the fault was not my own in the end.

[guestgulkan]

Your file outputs the NewName at certain points and the result is as follows: (which is correct for the way the program is wriiten and the way the output.txt file is arranged)

Maps\
Maps\Abbottsburg
Maps\Abbottsburg.tif
Maps\
Maps\o34078c2.tif
Maps\o34078c2.tif.tif

[ShadowTek]

1. remove the ios::binary because it sucks in the carriage returns at the ned of the lines and adds them to the lines/text

The only reason that I am reading it as a binary is because of the issue described in the following thread.
http://www.cplusplus.com/forum/beginner/3599/


Ok, I tried to debug a little, but now I have a new problem. I added some code that first checks and states the string size, then prints the string, and then prints each character of the string one by one. The problem is that I cant print the first character of the string from my loop, but I can print it just fine outside the loop.

He is the resulting printout:

String length is: 12
" " "b" "b" "o" "t" "t" "s" "b" "u" "r" "g" "

Abbottsburg
A
b

There must be something about string or char use that I don't understand. I don't get why I can't print out that first character, or why there is an extra double quote at the end of that individual character readout line.

Here is the complete code with the recent debug stuff added:

#include <stdio.h>
#include <iostream>
#include <fstream>
#include <string>
using namespace std;
char * MasterList = "OutputFile.txt";
bool ErrorOccurred = false;
string FilePath = "Maps\\";
string MapFileExt = ".tif";



int main ()
{
     string * MasterListLines;
     ifstream MasterListStream (MasterList, ios::binary);

     string TempString;
     int NumberOfLines = 0;

     if (MasterListStream.is_open())
     {
                                    while (! MasterListStream.eof())
                                          {
                                                                    getline (MasterListStream, TempString);
                                                                    ++NumberOfLines;
                                                                    }
                                                                    MasterListStream.close();
                                                                    }
     else cout << "Unable to open file.";
     
     MasterListLines = new string[NumberOfLines];
       
     MasterListStream.clear();
     MasterListStream.open (MasterList, ios::binary);
     if (MasterListStream.is_open())
     {
                                    while (! MasterListStream.eof())
                                    {
                                          for (int n=1; n<=NumberOfLines; ++n)
                                          {
                                              getline (MasterListStream, MasterListLines[(n-1)]);
                                              if (! MasterListStream.good() && ! MasterListStream.eof())
                                                 ErrorOccurred = true;
                                          }
                                     }
     }
     else
         cout << "Unable to open file.";
     if (ErrorOccurred == true)
       cout << "WARNING: SOME ERROR OCCURRED!"; 
       
     MasterListStream.close();
// THIS BEGINS THE AREA OF CONCERN.

//Start new debug. 
     int StringLength;
     for (int n=0; n<1; ++n)
     {
         StringLength = MasterListLines[n].length();
         cout << "String length is: " << StringLength << endl;
         for (int m=0; m<StringLength; ++m)
         {
             cout << "\"" << MasterListLines[n][m] << "\" ";
         }
         cout << endl << endl;
     }
     cout << MasterListLines[0] << endl;
     cout << MasterListLines[0][0] << endl;
     cout << MasterListLines[0][1] << endl << endl;
// End new debug.
     
     string NewName;
     string OldName;
     int OpResult;
     for (int n=0; n<NumberOfLines; n+=4) // This loop only runs twice.
     {
         NewName = FilePath;
         OldName = FilePath;
         NewName.reserve(512);
         OldName.reserve(512);
/*
           cout << NewName.size() << endl;
           cout << NewName.capacity() << endl;
*/
           cout << NewName << endl;
         NewName.append (MasterListLines[n]); /* <------ This Is what I want to use. This is what was used in the second example above. */
//         NewName.append ("Abbottsburg");    /* <----- For some reasom, using this instead will make it work. This is what was used in the first example above. */

/* 
           cout << NewName.size() << endl;
           cout << NewName.capacity() << endl;
*/
           cout << NewName << endl;
         OldName.append (MasterListLines[n+1]);
         NewName.reserve(512);
/*
           cout << NewName.size() << endl;
           cout << NewName.capacity() << endl;
*/
         NewName.append (MapFileExt);
/*
           cout << NewName.size() << endl;
           cout << NewName.capacity() << endl;
*/
           cout << NewName << endl;
           }
    delete[] MasterListLines;
    return 0;
}

This may all be some kind of screw up related to the fact that I am working with strings that I read in from a binary file stream, but I don't know what else to do, considering the unsolved problem in that other thread I referred to.

[guestgulkan]

Before we get too carried away and more confused..

Your output file text looks like this:
Abbottsburg
o34078e6.tif
http://www.archive.org/download/usgs_drg_nc_34078_e6/o34078e6.tif

Acme
o34078c2.tif
http://www.archive.org/download/usgs_drg_nc_34078_c2/o34078c2.tif


You read in the lines of the file into the MasterListLines array

You have 2 strings OldName and NewName.
What lines do you want to put into each of these two strings??
In other works what should NewName look like and what should OldName look like once we have finished?

[Zaita]

NewName = FilePath;
NewName += MasterListLines[n];

I've never seen any real use for string.reserve.

[ShadowTek]

NewName will eventually be...

Maps\Abbottsburg.tif

...as a result of the first iteration of the for loop, and then it well be...

Maps\Acme.tif

...after the iteration of the second loop.

This consists of three parts: "Maps\\", which is a constant, "Abbotsburg" or "Acme", which is read from OutputFile.txt , and ".tif", which is a constant.

Within each loop, I plan to create further operations on them, once I get all of this current mess straightened out.

OldName will be...

Maps\o34078e6.tif

...after the first loop iteration, and then it will be...

Maps\o34078c2.tif

...after the second iteration.

This consists of only two parts: the "Maps\\" constant, and "o34078e6.tif" or "o34078c2.tif", which is read from OutputFiles.txt .

But I'm not having any problems with OldName because I am only using append once for that. Append also works fine for NewName the first time that I use it, but it screws up the second time that I use append.

[ShadowTek]

I've never seen any real use for string.reserve.

Oh, I was just doing that because I thought this problem might be because of some kind of allocation problem, with append() not properly extending the string. So that was just debug stuff.

[guestgulkan]

Your original code was pretty much it - Like I mentioned earlier - text lines and binary file reading do not go well together.
here is the original code - try this and see how it works.
I assume you are

#include <stdio.h>
#include <iostream>
#include <fstream>
#include <string>
using namespace std;
char * MasterList = "OutputFile.txt";
bool ErrorOccurred = false;
string FilePath = "Maps\\";
string MapFileExt = ".tif";



int main ()
{
     string * MasterListLines;
     ifstream MasterListStream (MasterList);

     string TempString;
     int NumberOfLines = 0;

     if (MasterListStream.is_open())
     {
                                    while (! MasterListStream.eof())
                                          {
                                                                    getline (MasterListStream, TempString);
                                                                    ++NumberOfLines;
                                                                    }
                                                                    MasterListStream.close();
                                                                    }
     else cout << "Unable to open file.";
     
     MasterListLines = new string[NumberOfLines];
       
     MasterListStream.clear();
     MasterListStream.open (MasterList);
     if (MasterListStream.is_open())
     {
                                    while (! MasterListStream.eof())
                                    {
                                          for (int n=1; n<=NumberOfLines; ++n)
                                          {
                                              getline (MasterListStream, MasterListLines[(n-1)]);
                                              if (! MasterListStream.good() && ! MasterListStream.eof())
                                                 ErrorOccurred = true;
                                          }
                                     }
     }
     else
         cout << "Unable to open file.";
     if (ErrorOccurred == true)
       cout << "WARNING: SOME ERROR OCCURRED!"; 
       
     MasterListStream.close();
// THIS BEGINS THE AREA OF CONCERN.
     string NewName;
     string OldName;
     int OpResult;
     for (int n=0; n<NumberOfLines; n+=4) 
     {
         NewName = FilePath;
         OldName = FilePath;

          cout << NewName << endl;
         NewName.append (MasterListLines[n]); 

          cout << NewName << endl;
         
          OldName.append (MasterListLines[n+1]);
        
         NewName.append (MapFileExt);

           cout << NewName << endl;

           /* I assume you are going to do something with the filename here */
     }


    delete[] MasterListLines;
    return 0;
}

[Duthomhas]

I'm not entirely sure, but it looks like the compiler is having trouble with the way you are managing memory. Also, you are doing stuff twice when you only need to do it once. You won't save any memory by avoiding vectors or deques. Use them.

#include <deque>
#include <fstream>
#include <iostream>
#include <string>
using namespace std;

const char * MasterList = "OutputFile.txt";
string FilePath = "Maps\\";
string MapFileExt = ".tif";

int failure( const string& message )
  {
  cerr << message << endl;
  return 1;
  }

int main ()
{
     // Read the file contents
     deque <string> MasterListLines;
     ifstream MasterListStream (MasterList);
     if (!MasterListStream)
          return failure( "Unable to open file." );

     string TempString;
     while (getline( MasterListStream, TempString ))
     {
          MasterListLines.push_back( TempString );
     }
     int NumberOfLines = MasterListLines.size();

     if (!MasterListStream.eof())
          failure( "WARNING: SOME ERROR OCCURRED!" );

     MasterListStream.close();


     // Use the contents to modify my filenames
     for (int n=0; n<NumberOfLines; n+=4)  // loops once for every four lines of input
     {
          // output old name
          cout << (FilePath +MasterListLines[n+1]) << endl;

          // output new name
          cout << (FilePath +MasterListLines[n] +MapFileExt) << endl;

          // separate stuff visually
          cout << endl;
     }

    return 0;
}

I'm not sure exactly what you are trying to output, but the example I posted writes first the old filename then the new filename. I also stuck a blank line in there. The important point is that I used the + operator (which uses the .append() function) to stick stuff together.

Also notice how I used the deque to make things simple. It doesn't cost you any significant extra space (maybe 20 bytes or so) and avoids problems with dinking with pointers.

I would also like to draw your attention to line 26. This attempts to get a line (and does so if it can), then returns the ifstream object. The ifstream is then compared as a boolean value (which is the same as using .good()) to check for any stop condition (fail, bad, or eof). I could have written it as:
while (getline( MasterListStream, TempString ).good())

I don't use Dev-C++, but there should (hopefully) be an option in there somewhere to use "smart tabs". That will help clean up all the mis-aligned code.

Phew. Hope this helps.

Oh yeah, before I forget, .reserve() is useful if you know how much memory you will need before you actually use it, so that the string class doesn't have to do any fancy memory swapping whenever the string grows too large. That's not usually a problem, but if you need to optimize heavily for speed and/or size it is wonderful.

[edit] I think the reason he was having trouble is that opening the file in binary then reading to '\n' left him with a string ending in a '\r', which causes the 'print head' as it were, to return to the left side of the display. Hence, the string appears to be missing the first part, since the first part is shorter than the second. If the second were shorter, the resulting output would look confused.

[ShadowTek]

Great, thanks for taking the time to do that. It works perfect now and looks a hell of a lot better than that hobbled crap that was creating.

I seems that the whole basic problem all along were unwanted artifacts that I put into the string array by reading from that file in binary mode. Knowing now what the problem is and how to avoid it will help me to avoid problems in the future. I hate having unexplainable problems and not being able to figure things out.

I had included <stdio> because I was going to use the rename() function. That's the eventual goal of all of this. I have a large collection of topographic maps, but they are all named like "o34078e6.tif". I do a lot of outdoors stuff, and I look through my topos frequently. But, being named as they currently are, I have to refer to an index file, look up the literal name, then the cryptic numbered name, and then search for that. Renaming the entire collection would make researching through them a lot easier.

I generated the OutputFiles.txt to contain the current names of these files, preceded by the literal quad name of the topo. NewName contains the literal name of the map, and OldName contains the old, cryptic name of the map. This code was to help me prepare for the process of swapping out the names for the files.

I wasn't previously familiar with the uses of vector and deque. I just recently finished this site's tutorials, so I have been getting by using whatever methods and concepts were introduced there and whatever things I could pick up from Google searches about specific problems.

Also, I was under the impression that going through the process of explicitly defining the needed size of an array would be the best practice for making efficient code. And that is something that I can do in this case since the size of the array need only match the number of lines of the document that would be read from.

I know its not a big deal in this situation. I'm just trying to start good habits where I can.

Although, after reading about he difference in efficiency performance between vectors and deques, I like that deques don't reallocate the entire memory block on growth. Considering that, it doesn't seem bad at all. And It would probably be a more process efficient to use that, as opposed to taking the time to read through OutputFile.txt at the start, just to build a line count.

ALRIGHT, problem solved! Thanks everybody!

[Zaita]

Also,

OldName.append (MasterListLines[n+1]);

should be

OldName = OldName.append (MasterListLines[n+1]);

[Duthomhas]

That just invokes a self-assignment. The first way is fine. :-O