Reputation: 27
I found a solution but I believe there are better ways to do it. How can I improve my code without using complicated tools?
string Read(string& file) {
ifstream in;
string text;
string s;
in.open(file, ios::in);
try {
while (!in.eof()) {
text.append(s);
in >> s;
}
}
catch (exception& ex) {
cout << ex.what();
}
in.close();
return text;
}
Upvotes: 0
Views: 4188
Reputation: 136515
Your code reads whitespace-separated words, discards the whitespace, and concatenates all words into one string with no whitespace. You probably want to read the file content verbatim.
One way to read an entire file into a std::string
without a loop is to use std::string
constructor which takes two iterators - the constructor does the loop for you. Invoke it with std::istreambuf_iterator
:
#include <string>
#include <fstream>
#include <iterator>
#include <stdexcept>
std::string read(std::string filename) {
std::ifstream file(filename, std::ios_base::binary | std::ios_base::in);
if(!file.is_open())
throw std::runtime_error("Failed to open " + filename);
using Iterator = std::istreambuf_iterator<char>;
std::string content(Iterator{file}, Iterator{});
if(!file)
throw std::runtime_error("Failed to read " + filename);
return content;
}
Another alternative is to map the file into memory (zero-copy method), e.g. using boost::iostreams::mapped_file
, that's as clean and efficient as it ever gets. The method is faster for files larger than ~100kB, benchmark to get hard numbers.
An optimization would be to populate all the pages of the file mapping immediately, rather than with demand paging on first access.
Example:
#include <iostream>
#include <boost/iostreams/device/mapped_file.hpp>
int main() {
using boost::iostreams::mapped_file;
mapped_file content("/etc/lsb-release", mapped_file::readonly);
std::cout.write(content.const_data(), content.size());
}
Upvotes: 5
Reputation: 88017
The simplest way (which often times means the best way) is to read the file one character at a time
string text;
char ch;
while (file.get(ch)) {
text += ch;
}
Upvotes: 2
Reputation: 25386
The loop
while (!in.eof()) {
text.append(s);
in >> s;
}
is wrong, because the condition while (!in.eof())
may be false
even when the previous statement in >> s
succeeded. It would be better to write while (!in.fail())
instead of while (!in.eof())
.
However, it would be clearer and slightly more efficient to write the loop like this:
while ( in >> s ) {
text.append(s);
}
This loop condition indirectly uses in.operator bool()
, which is equivalent to !in.fail()
.
The reason why this loop is a bit more efficient than the other loop is that in the first iteration of the other loop, an empty string was always appended. This is no longer the case in this loop.
Although you did not state this in the question, in the comments section of your question, you stated that it would be preferable for the whitespace characters to be retained instead of being discarded. In that case, you should not be using operator >>
for reading input, which reads one word at a time, but you should rather be using std::getline
, which reads one line at a time:
string Read(string& file) {
ifstream in;
string text;
string s;
in.open(file, ios::in);
while ( getline( in, s ) ) {
text.append( s );
text.append( "\n" );
}
return text;
}
Upvotes: 1