Read text file into std::string.

J

Jason Heyes

Does this function need to call eof after the while-loop to be correct?

bool read_file(std::string name, std::string &s)
{
std::ifstream in(name.c_str());
if (!in.is_open())
return false;

char c;
std::string str;
while (in.get(c))
str += c;

if (!in.eof())
return false;

s = str;
return true;
}

Thanks.
 
V

velthuijsen

Jason said:
Does this function need to call eof after the while-loop to be correct?

bool read_file(std::string name, std::string &s)
{
std::ifstream in(name.c_str());
if (!in.is_open())
return false;

char c;
std::string str;
while (in.get(c))
str += c;

if (!in.eof())
return false;

s = str;
return true;
}

Calling EOF after the loop here is a check to insure that you exited
the loop because you read to the end of the file. There are a few other
reasons that get() might not be able to read a char and return a count
of 0.
 
R

Rapscallion

Jason said:
Does this function need to call eof after the while-loop to be correct?
Yes.

bool read_file(std::string name, std::string &s)
{
std::ifstream in(name.c_str());
if (!in.is_open())
return false;

char c;
std::string str;
while (in.get(c))
str += c;

This loop is _extremely_ slow (or do you want to prove the
impracticality of C++?). Use getline (in, str) instead.
 
M

Me

Does this function need to call eof after the while-loop to be correct?

Depends. The stream can terminate that while loop for a number of
reasons including a bad harddrive or an out of memory condition, etc.
If you want to make sure it read all the characters all the way to end
of the file, you have to check if it exited the loop because due to
eof.
bool read_file(std::string name, std::string &s)
{
std::string str;


btw, if this is your code, I suggest you do s.swap(str) instead of the
assignment.
 
P

Peter Julian

Jason Heyes said:
Does this function need to call eof after the while-loop to be correct?

bool read_file(std::string name, std::string &s)
{
std::ifstream in(name.c_str());
if (!in.is_open())
return false;

char c;
std::string str;
while (in.get(c))
str += c;

if (!in.eof())
return false;

s = str;
return true;
}

Thanks.

Why not extract the file's contents into a container of strings? If you make
the container a member of a class, read_file() and a write_file() could be
member functions of that class with access to the encapsulated container.

There are a number of other ways this could be done. You might overload the
write() member function so it takes a reference to another string container
to write to file, you might choose to use the same filestream to read /
write, etc.

// FileParser.cpp
//
#include <iostream>
#include <string>
#include <fstream>
#include <vector>
#include <algorithm>
#include <stdexcept>

class FileParser
{
// members
const std::string m_filename_;
std::string m_buffer;
std::ifstream m_ifs;
std::eek:fstream m_ofs;
std::vector<std::string> m_vs;
public:
// ctor
FileParser(std::string s)
: m_filename_(s), m_buffer(), m_ifs(), m_ofs(), m_vs() { }
~FileParser() { }

// read() member function
void read() throw(std::exception)
{
m_ifs.open(m_filename_.c_str(), std::ios::in);
if (!m_ifs)
{
throw std::exception("error opening file for read().\n");
}
while (std::getline(m_ifs, m_buffer))
{
m_vs.push_back(m_buffer);
}
if (!m_ifs.eof()) // if reason of termination != eof
{
throw std::exception("error while parsing file.\n");
}
}

// write(...) member function
void write() throw(std::exception)
{
m_ofs.open(m_filename_.c_str());
if (!m_ofs)
{
throw std::exception("error opening file for write().\n");
}
// copy algorithm with ostream_iterator
std::copy( m_vs.begin(),
m_vs.end(),
std::eek:stream_iterator<std::string>(m_ofs, "\n") );
}

// load(...) member function
void load(std::vector<std::string>& r_vs)
{
m_vs.clear();
std::copy(r_vs.begin(), r_vs.end(), std::back_inserter(m_vs));
}

// display() member function
void display() const
{
std::cout << "\n--- FileParse::display() ---\n\n";
std::copy( m_vs.begin(),
m_vs.end(),
std::eek:stream_iterator<std::string>(std::cout,
"\n") );
}

}; // class FileParser

int main()
{
std::vector<std::string> vstrings;
vstrings.push_back("string 0");
vstrings.push_back("string 1");
vstrings.push_back("string 2");
vstrings.push_back("string 3");
vstrings.push_back("string 4");

// FileParser object
FileParser fileparser("file.dat");
fileparser.load(vstrings);
fileparser.display();

try
{
fileparser.write();

fileparser.read();
fileparser.display();
}
catch (const std::exception& e)
{
std::cout << "Error:\n";
std::cout << e.what();
}

return 0;

} // main()

/* file.dat

--- FileParse::display() ---

string 0
string 1
string 2
string 3
string 4

*/
 
J

Jason Heyes

Rapscallion said:
This loop is _extremely_ slow (or do you want to prove the
impracticality of C++?). Use getline (in, str) instead.

Why is the loop slow? Isn't get inlined?

I read that getline needs a count of the number of elements to extract and
an array to store them. It does not take std::string. So how can I use
getline instead of get in this function?
 
V

velthuijsen

Jason said:
Why is the loop slow? Isn't get inlined?

I read that getline needs a count of the number of elements to extract and
an array to store them. It does not take std::string. So how can I use
getline instead of get in this function?

getline(in, str);

This reads the data until it finds either EOF, '\n' or max amount of
data that can be read is read (to check the last you need to see if the
fail bit is set).
It doesn't copy the '\n' (or EOF) into the string but it does extract
it so the code should be:
getline(in, str);
str+= '\n';
 
R

Rapscallion

Peter said:
Why not extract the file's contents into a container of strings? If you make
the container a member of a class, read_file() and a write_file() could be
member functions of that class with access to the encapsulated container.

There are a number of other ways this could be done. You might overload the
write() member function so it takes a reference to another string container
to write to file, you might choose to use the same filestream to read /
write, etc.

Your FileParser is done well but the design is not perfect. Some
remarks are attached. Refactoring the design is recommended.
// FileParser.cpp
//
#include <iostream>
#include <string>
#include <fstream>
#include <vector>
#include <algorithm>
#include <stdexcept>

class FileParser
{
// members
const std::string m_filename_;
std::string m_buffer;
std::ifstream m_ifs;
std::eek:fstream m_ofs;

These need not be members.

std::vector<std::string> m_vs;

Use std::list to avoid unnecessary copies of strings (each copy > 16
means dynamic allocation on VC 7.1)
std::vector said:
public:
// ctor
FileParser(std::string s)
: m_filename_(s), m_buffer(), m_ifs(), m_ofs(), m_vs() { }
~FileParser() { }

// read() member function
void read() throw(std::exception)
{
m_ifs.open(m_filename_.c_str(), std::ios::in);
if (!m_ifs)
{
throw std::exception("error opening file for read().\n");
}
while (std::getline(m_ifs, m_buffer))
{
m_vs.push_back(m_buffer);

m_vlist.resize (m_vlist.size() +1);
m_vlist.back().swap (m_buffer);
}
if (!m_ifs.eof()) // if reason of termination != eof
{
throw std::exception("error while parsing file.\n");
}
}

// write(...) member function
void write() throw(std::exception)

void write (std::eek:stream& os) // write to any ostream
{
m_ofs.open(m_filename_.c_str());
if (!m_ofs)
{
throw std::exception("error opening file for write().\n");
}
// copy algorithm with ostream_iterator
std::copy( m_vs.begin(),
m_vs.end(),
std::eek:stream_iterator<std::string>(m_ofs, "\n") );
}

// load(...) member function
void load(std::vector<std::string>& r_vs)

why this???
{
m_vs.clear();
std::copy(r_vs.begin(), r_vs.end(), std::back_inserter(m_vs));
}
// display() member function
void display() const

friend
std::eek:stream& operator<< (std::eek:stream& os, const FileParser& f);
 
R

Ron Natalie

Jason said:
Why is the loop slow? Isn't get inlined?
It's inlined probably, but still moves one character at a time
fom the stream and grows the string one element at a time with it.
I read that getline needs a count of the number of elements to extract and
an array to store them. It does not take std::string. So how can I use
getline instead of get in this function?
You didn't read far enough. There's a getline that's not a member
of stream that takes a string.
 
R

Rolf Magnus

Ron said:
It's inlined probably, but still moves one character at a time
fom the stream and grows the string one element at a time with it.

And you think getline does someting else internally?
 
J

Jason Heyes

getline(in, str);

This reads the data until it finds either EOF, '\n' or max amount of
data that can be read is read (to check the last you need to see if the
fail bit is set).
It doesn't copy the '\n' (or EOF) into the string but it does extract
it so the code should be:
getline(in, str);
str+= '\n';

Do you mind writing the loop as well? I can't see what you are doing.
You get an extra newline character added to the end of str with that code.
 
V

velthuijsen

Do you mind writing the loop as well? I can't see what you are doing.
You get an extra newline character added to the end of str with that code.

It wasn't meant as a complete function, just a comment on how to work
with it.
And yes you need that extra newline since the getline function doesn't
add it to the string but it does remove it from the stream.
But if you want a while loop, here is the whole function.

bool read_file(std::string name, std::string &s)
{
std::ifstream in(name.c_str());
if (!in.is_open())
{
return false;
}

std::string str;
while(in.good())
{
// guard against reading '\n' and then EOF
if (std::char_traits<char>::eof() != in.peek())
{
break;
}
getline(in, str);
str+= '\n';
}
if (!in.eof())
{
return false;
}
s.swap(str);
return true;
}
 
P

Peter Julian

Rapscallion said:
Peter Julian wrote:


These need not be members.

I can't argue the point since you said "need not". I prefer these to be
members.
Use std::list to avoid unnecessary copies of strings (each copy > 16
means dynamic allocation on VC 7.1)
std::vector<std::string> m_vlist;

To keep it readable for the OP, i used the simple sequential vector. If i
chose to provide a better container than the vector, i would have
implemented a std::deque (think iterator-type).

void write (std::eek:stream& os) // write to any ostream

This is a good idea and it warrants implementation using an oveloaded or
alternate member function. The strategy behing write() here is to serialize
the encapsulated container's contents to file, not to be dependant on a
parameter.
But i definitely like the suggestion.

why this???

Why not? It provides the class with the ability to populate the string
container without accessing a file.
friend
std::eek:stream& operator<< (std::eek:stream& os, const FileParser& f);

Yes, that too is a valid suggestion. display() is included here only to keep
the interface simple for the OP.

Thanks for the input.
 
S

Shezan Baig

Rapscallion said:
Just don't add charactes one by one to the string. Do it in larger
chunks.


Or how about just:

std::string text;

text << fileStream.rdbuf();

This seems most straightforward to me. No need to mess with loops and
what not.

-shez-
 
R

Rapscallion

Shezan said:
Or how about just:
std::string text;

text << fileStream.rdbuf();

This seems most straightforward to me. No need to mess with loops and
what not.

The 'mess with loops' and 'text << fileStream.rdbuf()' produce the same
result? Really?
 
J

Jason Heyes

Shezan Baig said:
Or how about just:

std::string text;

text << fileStream.rdbuf();

This seems most straightforward to me. No need to mess with loops and
what not.

How can that work? It looks like you were trying to write:

is.rdbuf() >> text;

This does not work.
 
S

Shezan Baig

Jason said:
How can that work? It looks like you were trying to write:

is.rdbuf() >> text;

This does not work.


Sorry, wasn't thinking. It should be:

std::stringstream ss;

ss << is.rdbuf();

now you can do whatever you want with 'ss.str()'.

hth,
-shez-
 

Ask a Question

Want to reply to this thread or ask your own question?

You'll need to choose a username for the site, which only take a couple of moments. After that, you can post your question and our members will help you out.

Ask a Question

Members online

Forum statistics

Threads
473,743
Messages
2,569,478
Members
44,899
Latest member
RodneyMcAu

Latest Threads

Top