I've made a program which reads from the console and writes it in a .txt
file. This programm should write in a file with \n
.
#include <iostream>
#include <fstream>
#include <string>
using namespace std;
int main() {
ofstream fich("file.txt");
if (!fich)
cerr << "Error!\n";
char fst = 'l';
string c;
while (fst != '0円') {
getline(cin, c);
fst = c[0];
fich << c << endl;
}
fich.close();
return 0;
}
This program ends when the user inputs nothing and then a \n
, because, doing that, the string c
will only have a 0円
end of string element.
I was wondering if there is any way of doing this more efficiently, maybe without needing a "void \n
" line to finish the program.
2 Answers 2
Error Handling
You check whether the output file was opened correctly, but all you do is output a message (you don't even say what's wrong!) and then you keep on ignoring it. What if file.txt
is a directory or read-only? I guess you haven't tested these cases. By default, throw an exception if you can't continue at some point. In this case, throw std::runtime_error("failed to open output file")
.
Loop Condition
You are reading until the line is empty. This is okay-ish, but why don't you just check whether c.empty()
and then break from the loop? Another case is that the end of the input is reached. In that case, the streamstate is set to failed. I'm not sure what happens in getline()
then (read the docs at cppreference.com), but if it just doesn't change the string passed by reference, you may end up in an endless loop.
As an alternative I would use getline(cin, c)
as loop condition. If this succeeds, you have received a line which can then write to the output file. Otherwise, you either had an error or you reached the end of the input.
Closing Filestreams
If you didn't call fich.close()
, when would it be closed? However, consider the possibility that the file can't be written fully. You'll never notice this, neither with your code nor if you let the destructor do its work. So, what I'd rather do there is a call to fiche.flush()
followed by a check of the streamstate.
Efficiency
I wouldn't bother with that at the moment. Point is, you're still learning to walk, so don't try to run yet. Still, what you're doing is inefficient. Firstly, writing in size of lines is useless, because you're just moving bytes from one file to the other. Secondly, endl
implies a newline and a flush. So this is not really a cheap operation! In addition, there is a lot of stuff going on in C++ IOStreams that there's a whole book about their internals. Lastly, copying files can sometimes be done more efficiently using OS features which will avoid looking at the data more than necessary.
-
\$\begingroup\$ Yes a file will be closed correctly. The
std::ofstream
destructor calls close. In "most" cases not calling close is the correct action. Also manually calling flush is "usually" the wrong thing to do. The system implementation will auto flush when needed; manually calling flush is the cause of most speed issues in beginners C++ application. \$\endgroup\$Loki Astari– Loki Astari2021年05月26日 22:41:42 +00:00Commented May 26, 2021 at 22:41 -
\$\begingroup\$ Speed issues in beginners' applications? Uselessly flushing is a source for bad performance, that much I agree with. However, if you don't flush your stream after you're done writing, you don't know whether the data was written. If there is buffered data that doesn't fit on the storage, you will only get corrupted data and you may not even notice until it's too late. \$\endgroup\$uli– uli2021年05月27日 06:48:28 +00:00Commented May 27, 2021 at 6:48
-
1\$\begingroup\$ "I'm not sure what happens in getline() then (read the docs at cppreference.com), but if it just doesn't change the string passed by reference, you may end up in an endless loop." The string is cleared first. So if there is an EOF or other problem, you will read an empty string. \$\endgroup\$JDługosz– JDługosz2021年05月27日 13:48:52 +00:00Commented May 27, 2021 at 13:48
The 0円
does not show up as a legal element of the string
. In an empty string
, doing c[0]
is an error!
You don't need fst
at all. You should use c.empty()
to check for an empty string.
If you're not on a Unix-like system, be sure to open the file in Text mode, or you'll end up with stray \r
characters in your string.
Your names, fst
, fich
, are confusing. Using c
for a string is unusual and confusing to experienced programmers.
Don't write using namespace std;
.
You can use a mid-decision loop to simplify your variables and logic. Something like:
for (;;) {
getline (cin, s);
if (s.empty()) break;
outfile << s << '\n';
}
As as already been pointed out, the whole idea of reading and writing a line at a time is not "efficient". It might be what you need, though, for an interactive program where the user is typing something. If you are implementing something like cat
though it is less efficient than reading entire blocks using low-level primitives. The iostream
implementation reads the file in a low-level way, and getline
goes through that data (that's already been read into memory) just to locate the end-of-line characters; then copies that to another string. If you just wanted to copy everything, that's clearly extra overhead.
main2()
is a typo that should bemain()
, right? \$\endgroup\$c[0]
on an empty string is not guaranteed to be0円
! Your test should be:while(getline(std::cin, c)) {
Then you can end input by typing the EOF control code for your platform.<ctrl>-D
on linux. This also makes it work nicely with files piped to the standard input. \$\endgroup\$0円
element should be instring[0]
. \$\endgroup\$std::string
class does not use a null terminator. Accessing atsize()
usingoperator[]
is undefined behavior. Now there is a method that will give you a null terminated string that looks like a C-String callc_str()
this guarantees a null terminated string. But why not just callempty()
to test for an empty string. \$\endgroup\$