Quote:Original post by ShadowBranch
just to add my bit to this. This is th best algorithm I use for reading from files and then later writing to files.
- You don't use anything from <cstring> in the illustrated program (thank goodness), so don't include it. (Technically, I don't use iostream below, but it's almost certain to be used in a full program, whereas you would only need to include cstring if you were using stuff like strlen() etc., which you really shouldn't be using in normal circumstances.)
- Declaring a parameter as 'char File[255]' is rather silly: it
doesn't actually limit the input, and there is no reason to do so anyway. It's more useful to accept a string by const reference and convert it with .c_str(); a passed-in string literal then gets converted back and forth, but it will still work - whereas you can't pass a std::string to a function expecting const char*.
- Don't use .open() on an fstream that you just declared. Instead, use the constructor that takes the .open() arguments. Remember, initialization != assignment, and you should initialize variables to their initial value when you know what that is.
- is_open() returns a bool. You should treat bools as if they were, you know, boolean: don't compare them to literals in order to answer 'if' conditions, because they already are the type that fundamentally represents the answer to a yes-or-no question.
- Use ifstream and ofstream to be specific; then ios::in and ios::out respectively are implied.
- Don't put parentheses around the things you're going to return; 'return' is not a function but a keyword.
- It might be a good idea to actually return the stuff that is read from a file, so that something can be done with it. Doing all the processing inside ReadFile() would be horrendously poor code organization.
- Don't handle errors by printing a local message and then returning a status value. If you're determined to indicate the error with a status value, you should let the calling code print the message as part of its error handling. Here, however, I would recommend either throwing an exception, or just returning an empty data set.
- Use the
'while (reading operation)' idiom for looping over input files.
- There's no reason to invoke .write() to output characters, especially if all the other I/O is using formatted I/O via the operator<<. This operator is overloaded to handle chars, including character literals (such as '\n' - notice *single* quotes) in the way you would expect it to. Note that writing 2 bytes for "\n" would also write the null terminator into the file, which is often not what you want (although probably benign).
#include <iostream>#include <fstream>#include <string>#include <vector>#include <exception>using namespace std;vector<string> ReadFile(const string& name){ ifstream theFile(name.c_str()); if (!theFile.is_open()) { throw runtime_error("File not found"); } vector<string> result; string line; while (getline(theFile, line)) { result.push_back(line); } return result;}
