0

I am trying to open many files in a row. The file names are all stored in a vector, which is passed to my function. This is just a simple test to make sure that everything is working. If it works, then I will need to pushback whatever the file holds into another vector.

void readfile(vector<string> &filename)
{
    string temp;
    ifstream infile(filename[2].c_str());
    getline (infile, temp);
    cout << temp << endl;
}

This is simply outputting a blank line, although the text file holds about a paragraph of information. I have not used File I/O in a while, so I am a little rusty. Any help is appreciated.

Edit: You all have been a great help, one more thing, I need to pass them without periods, or spaces. Basically just a string of chars.

Bob Benson
  • 67
  • 2
  • 6

2 Answers2

1

OP's code does not check for success of any of their file IO, so the file may not have opened, the file may be empty, and the read may have failed for any number of reasons.

Fortunately, getline returns the input stream and streams implement a really neat operator bool that returns false if the stream is in an error condition and could not be read. If the file could not be read, the contents of temp are most certainly not valid and should not be used.

So...

void readfile(vector<string> &filename)
{
    string temp;
    ifstream infile(filename[2].c_str());
    if (getline (infile, temp)) //test the read for success
    {
        cout << temp << endl;
    }
    else
    {
        cout << "failed to read file" << endl;
    }    
}

If getline fails for any reason, including file not open, file is empty, and file is corrupt and unreadable, the stream's state will be marked bad and return false when checked by if().

Normally at this point you should check the type of error, infile.clear() the stream to remove the error condition, and pick up the pieces, but in this case there isn't much point. If you can't read the beginning of a file into a string, you got big problems and should take a closer look at the health and contents of file filename[2].

By the way, if your compiler is relatively up to date, ifstream's constructor will eat a std::string and ifstream infile(filename[2]); will be valid.

Style-wise, you're better off just passing the filename string into readfile, not the vector. This allows you to reuse the readfile function for more than just element 2 of a vector.

void readfile(string & filename)
{
    string temp;
    ifstream infile(filename);
    if (getline (infile, temp)) //test the read for success
    {
        cout << temp << endl;
    }
    else
    {
        cout << "failed to read file " << filename << endl;
    }    
}

And call with

readfile(filename[2]);

Expanding this function to meet OP's true goal

void readfile(string & filename,
              vector<string> & strings)
{
    string temp;
    ifstream infile(filename);
    if (getline (infile, temp)) //test the read for success
    {
        strings.push_back(temp);
        cout << temp << endl;
    }
    else
    {
        cout << "failed to read file " << filename << endl;
    }    
}

and call with

vector<string> strings;
...
readfile(filename[2], strings);
user4581301
  • 33,082
  • 7
  • 33
  • 54
  • Would this push back the string without spaces and periods? – Bob Benson Oct 26 '15 at 23:22
  • No. That would require a bunch of string manipulation covered very well in the selected answer here: http://stackoverflow.com/questions/5891610/how-to-remove-characters-from-a-string . Use something along those lines to remove the characters you don't want from `temp`, then `push_back`. – user4581301 Oct 26 '15 at 23:38
0

Your function assumes that there are at least three elements in your vector of file names and ignores all filenames except the third (element 2).

Also, the call to getline() only reads the first line of the file -- which may be a blank line.

I recommend you write a function to extract the contents of a single file, and use it in a loop (in another function?) to handle each file name in your list of filenames.

Relatedly, 'filename' implies a single filename; 'filenames' would better indicate that your variable is multiple file names.

// Return file content as a single string
string readfile( const string& filename )
{
  ifstream f( filename );
  stringstream ss;
  ss << f.rdbuf();
  return ss.str();
}

// Return file content as a vector/deque/whatever of strings
deque<string> readfile( const string& filename )
{
  ifstream f( filename );
  deque<string> lines;
  string line;
  while (getline( f, line )) lines.emplace_back( line );
  return lines;
}

Errors are not reported. (You'll just have an empty string/vector/deque/etc.)

Use it elsewhere in a loop. Here I've got a map of file name->file data, loading each file name in the list of file names:

for (const string& filename : filenames)
  filedata[ filename ] = readfile( filename );

Hope this helps.

Dúthomhas
  • 8,200
  • 2
  • 17
  • 39
  • The function I have that reads the filenames into a vector works perfectly, could i pass filenames[k].c_str() to the first function, have it open the specific file, then push_back what it returns into another vector? – Bob Benson Oct 26 '15 at 22:51
  • Absolutely. Also, pay attention to @user4581301's answer below, as he touches on noticing specifically when things go wrong, which I glossed over here. – Dúthomhas Oct 26 '15 at 23:25