tags:

views:

268

answers:

7

I have a random piece of code, I use for reading from CSV files... and it's fine... until after about 2000 reads... then the getline line fails with an access violation to 0xcccccc... which I assume means that the input stream (file) has been cleared... Not that I know why :)

int CCSVManager::ReadCSVLine ( fstream * fsInputFile,
                               vector <string> * recordData )
{
    string s;

    getline ( *fsInputFile, s );
    stringstream iss( s );

    for ( unsigned int i = 0; i < getNumFields (); i++ )
    {
        getline ( iss, s, ',' );
        (*recordData)[i] = s;
    }

    return 0;
}

Any ideas why?

+3  A: 

Are you sure that recordData has enough space for holding all records? It should look like this:

for ( unsigned int i = 0; i < getNumFields() && i < recordData->size() && getline(iss, s, ','); i++ ) 
 { 
  (*recordData)[i] = s; 
 }
Kirill V. Lyadvinsky
+2  A: 

You may be corrupting memory if (*recordData).size() < getNumFields(). Consider eliminating getNumFields and relying on (*recordData).size() to store that information. Or, don't preset the size of the vector at all and use push_back.

Avoid the pointers, too, with references.

int CCSVManager::ReadCSVLine ( fstream &fsInputFile, vector <string> &recordData )
{
 string s;

 getline ( fsInputFile, s );
 istringstream iss( s );

 for ( unsigned int i = 0; i < getNumFields (); i++ )
 {
  getline ( iss, s, ',' );
  recordData.push_back( s );
 }

 return 0;
    }
Potatoswatter
+1 for references instead of pointers. (The +1 will take effect in 5 hours.)
GMan
Was about 8 hours, but whatev. :)
GMan
A: 

is getNumFields (); changing. That looks like its being recalculated everytime. Move that out side of loop

John Nolan
+5  A: 

std::vector does not reallocate itself to expand when you access it like an array like that. What you should be doing instead of

(*recordData)[i] = s;

is

recordData->push_back(s);

This will expand the vector as necessary. One important difference between the two methods is that the first one will always start writing from the first element of the vector. The second one will start appending to the end of the vector, which is different if the vector is not initially empty.

Tyler McHenry
With a recordData->clear() as the first operation yes, otherwise the vector size will continue to grow
Richard Harrison
A: 

My guess would be that getNumFields() is returning a number greater than the number of lines in the file.

How many lines/records does the file contain?

What does getNumFields() return?

ChrisF
A: 

Argh... All very good answers but it was actually something far, far, far sillier. Thanks to your answers I started looking to some "other" parts of code and I realised what I had done wrong :) (It was actually the file being passed in that was blank. I didn't realise it was re-opening the file every time and overloading the file vector... such a stupid mistake :))

Thanks for your answers :) Still got me to my solution :)

Micheal
+1  A: 

a call as follows before the for statement to ensure that the vector has enough elements.

recordData->resize(getNumFields ()); 
Richard Harrison
You mean `resize`.
UncleBens
I did mean resize thanks for pointing it out.
Richard Harrison