C++ Code Dumping "random" data

Question

I will start by saying I am reasonably new as a C++ programmer. However I understand PHP and VBA, so have a good understanding of the aspects of programming fundamentals.

Because I use CSV's quite often in my day to day job, I thought it would be a good learning exercise to write a library that manipulates CSV Files.

I wrote this function:

int getHeaders(ifstream & os, vector<string> & head2){
    string STRING;
    getline(os,STRING);
    cout << STRING << endl;
    STRING.erase(remove(STRING.begin(), STRING.end(), '\"'), STRING.end());
    string::iterator it = STRING.begin();
    int x = 0;
    for (int index = 0; it < STRING.end(); it++, index++) {
        if (*it == ',') {
            head2.push_back(STRING.substr(0,index));
            STRING.erase(0,index+1);
            cout << endl << head2[x];
            cout << endl << STRING;
            x++;
        }
    }
    return head2.size();
}

Which is called by the following:

int addRowCount = 0;
vector<string> head1;

ifstream outfile;
outfile.open("default.csv", ios_base::app);

cout << getHeaders(outfile, head1) << endl;
cout << head1[0] << endl << head1[1] << endl;

But when I run the program, the program just dumps a load of random rubbish to the console (and crashes the application) I am using windows so cannot use valgrind.

Does anyone know why this may be happening? Obviously this "dump" is not what I want the application to do. I am hoping someone can point out the part of my code which would make this happen.

Thanks in advance.

Related: http://stackoverflow.com/questions/23047052/why-does-reading-a-struct-record-fields-from-stdistream-fail-and-how-can-i-fi — πάντα ῥεῖ, Jun 17 '14 at 17:33
How do you feel this is related? I have used no structs. Please bear in mind I am new to C++ and just looking for some pointers? — ggle, Jun 17 '14 at 17:38
@rhysowen What does your program do when `default.csv` can't be found in the correct directory? — Drew Dormann, Jun 17 '14 at 17:38
@Drew Dormann At the moment, nothing as I am currently at "concept" stage. But "default.csv" does exist (infact, another part of this program created and wrote to it) — ggle, Jun 17 '14 at 17:39
@rhysowen _'How do you feel this is related?'_ Check this answer about abstraction for the reading of character delimited text-formats: http://stackoverflow.com/a/23070803/1413395 — πάντα ῥεῖ, Jun 17 '14 at 17:41
It is worth me noting that the function was working fine until I turned it from a `void` to an `int` that also modified a `vector` — ggle, Jun 17 '14 at 17:43
@rhysowen You could simply have used `string::find_first_of` instead of looping until you find a `','` character. — PaulMcKenzie, Jun 17 '14 at 17:49
@PaulMcKenzie thanks, will definitely look to utilise that function — ggle, Jun 17 '14 at 17:52

Dale Wilson · Answer 1 · 2014-06-17T19:16:17.017

When you call erase on a string, iterators into that string are invalidated, so it is an error to use it after the call to STRING.erase().

Hint: When you look at the documentation for a method on a class that supports iterators, keep an eye out for notes about invalidating iterators. On this page, for example, read the section titled Iterator validity

[not related to the answer, but a style issue: Using ALL CAPS for a variable name like STRING is generally considered bad style in C and C++. All caps names are used for #defined symbols]

score 0 · Accepted Answer · answered Jun 17 '14 at 18:34

0

So it turns out that head2[1] wasn't set so it was some sort of memory leak.

Here is the finished function after a few further amendments for the advice in the comments section:

int getHeaders(ifstream & os, vector<string> & head2){
    string STRING;
    getline(os,STRING);
    STRING.erase(remove(STRING.begin(), STRING.end(), '\"'), STRING.end());
    int strle = count(STRING.begin(),STRING.end(), ',') + 1;
    for(int x = 0; x != strle; x++){
        if (count(STRING.begin(), STRING.end(), ',') > 0) {
            head2.push_back(STRING.substr(0,STRING.find_first_of(',')));
        } else {
            head2.push_back(STRING.substr(0,STRING.length()));
        }
        STRING.erase(0,STRING.find_first_of(',')+1);
    }
    return head2.size(); 
}

answered Jun 17 '14 at 18:34

ggle

426
3
20

If people are going to downvote this answer could you at least tell me why? – ggle Jun 17 '14 at 19:26
If you are interested in improving your C++ skills, I would suggest posting your *working* code on [Code Review Stack Exchange](http://codereview.stackexchange.com/). Also, if you haven't already, I would take a look at the first snippet of code from [this answer](http://stackoverflow.com/a/1120224/3033711). It is a much cleaner way of doing what you are trying to do. – jliv902 Jun 17 '14 at 19:41
@jliv902 I wasn't aware of the Code Review Stack Exchange! Will definitely start using that. And also that link to that other question will prove very useful when I start to review the complexity of my functions. Cheers! – ggle Jun 17 '14 at 19:44

C++ Code Dumping "random" data

2 Answers2